这是indexloc提供的服务，不要输入任何密码
Skip to content

Gnucobol pushed to version 3.2 and implemented berkeley db support #20968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Gnucobol pushed to version 3.2 and implemented berkeley db support #20968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d6836ec
branch from before my changes
alexbodn Jul 28, 2024
6825d8d
workflow
alexbodn Jul 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/packages.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ on:
- master
- dev
- 'dev/**'
- 'gnucobol-db'
Copy link
Contributor

@truboxl truboxl Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not change the CI workflow file. You can also create a Pull Request in your own fork for CI testing.

paths:
- 'packages/**'
- 'root-packages/**'
Expand Down
27 changes: 19 additions & 8 deletions packages/gnucobol/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,31 @@ TERMUX_PKG_HOMEPAGE=https://gnucobol.sourceforge.io/
TERMUX_PKG_DESCRIPTION="A free/libre COBOL compiler"
TERMUX_PKG_LICENSE="GPL-3.0, LGPL-3.0"
TERMUX_PKG_MAINTAINER="@termux"
TERMUX_PKG_VERSION=3.1.2
TERMUX_PKG_REVISION=3
TERMUX_PKG_SRCURL=https://ftp.gnu.org/gnu/gnucobol/gnucobol-${TERMUX_PKG_VERSION}.tar.xz
TERMUX_PKG_SHA256=597005d71fd7d65b90cbe42bbfecd5a9ec0445388639404662e70d53ddf22574
TERMUX_PKG_DEPENDS="json-c, libgmp, libvbisam, libxml2, ncurses"
TERMUX_PKG_VERSION=3.2
TERMUX_PKG_REVISION=0

TERMUX_PKG_SRCURL=git+https://github.com/alexbodn/gnucobol-3.2.git
Copy link

@GitMensch GitMensch Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you really need to change the upstream tarball?
Note that as-is you very likely trash the timestamps in that way, forcing a recreation of many files which wouldn't be necessary normally.

Copy link
Author

@alexbodn alexbodn Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was your suggestion to put the last release.
but i'm serving it off my github until you'd fix the issues upstream.
the main file to change was configure, and a file containing a timestamp.
i had to run autoconfig, hadn't i?

Copy link
Author

@alexbodn alexbodn Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the termux people didn't like the huge diff created by configure change.

Copy link
Member

@TomJo2000 TomJo2000 Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the termux people didn't like the huge diff created by configure change.

My main issue with it is that it is unmaintainable.
It's gonna stop applying and need triage as soon as there is any update to the package.

Copy link
Member

@TomJo2000 TomJo2000 Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should take this opportunity to note that changing the source URL to your personal fork is a complete no go unless you have a good explanation for why that is necessary.

Copy link
Member

@TomJo2000 TomJo2000 Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can guarantee $TMPDIR to have a consistent value at build time.
We can't guarantee that users won't willfully break their environments.

Copy link

@GitMensch GitMensch Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I'd say let's give them the fun to have a broken GnuCOBOL that way and drop the patch.

Copy link
Author

@alexbodn alexbodn Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and the termux people didn't like the huge diff created by configure change.

My main issue with it is that it is unmaintainable. It's gonna stop applying and need triage as soon as there is any update to the package.

I may argue, that each package is targeting a specific version of upstream, with a very specific sha256sum.

i had to change things when switching from gnucobol 3.1.2 to 3.2, but had to extend the changes since important functionality failed in termux, initially with both upstream versions btw.

it's the same with debian packages.

Copy link
Member

@TomJo2000 TomJo2000 Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may argue, that each package is targeting a specific version of upstream, with a very specific sha256sum.

The TERMUX_PKG_SHA256 field is a security and integrity measure.
It helps ensure that package sources have not been tampered with or corrupted.
The binding to a specific version is a result of this, though somewhat incidental.

Copy link
Author

@alexbodn alexbodn Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case I'd say let's give them the fun to have a broken GnuCOBOL that way and drop the patch.

I think it's a pitty.
if upstream gives this fallback option, why not on termux too?
it's definitely and legitimately termux related.

TERMUX_PKG_GIT_BRANCH="termux"

TERMUX_INSTALL_DEPS=true

TERMUX_PKG_DEPENDS="json-c, libgmp, libdb, libxml2, ncurses"

TERMUX_PKG_EXTRA_CONFIGURE_ARGS="
--without-db
--with-json=json-c
--with-vbisam
--with-db
--with-json=json-c
--with-xml2
"

termux_step_pre_configure() {
local lp64="$(( $TERMUX_ARCH_BITS / 32 - 1 ))"
export COB_LI_IS_LL="${lp64}"
export COB_32_BIT_LONG="$(( 1 - ${lp64} ))"
export COB_HAS_64_BIT_POINTER="${lp64}"
Comment on lines 22 to 25
Copy link

@GitMensch GitMensch Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are those not correctly set by configure? Please verify and drop a comment on that in the build script.

Copy link
Author

@alexbodn alexbodn Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well i didn't touch these.
nor have i tried any pointers yet.
all i did was making core functionality :).


export BDB_HEADER=$(cat db_header.c |cc -I$TERMUX_PREFIX/include -P -E -| tail -n 1| sed -e 's/[\ \t]//g')
Copy link

@GitMensch GitMensch Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a comment here that this a workaround for not being setup correctly in cross-compiling contexts, "last checked GnuCOBOL 3.2"

Copy link
Author

@alexbodn alexbodn Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with cross compiling configure refused to run any program, hence i did the header autodetection before configure, and pass the result to configure.

Copy link
Author

@alexbodn alexbodn Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it may be the other long-size related exports do fulfill the same void as my call:
inability to call code while configuring for cross compilation.

}

termux_step_post_configure() {
touch bin/cobcrun.1 cobc/cobc.1
}
11 changes: 0 additions & 11 deletions packages/gnucobol/configure.patch
View file
Open in desktop
Copy link

@GitMensch GitMensch Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this patch is correct to be deleted when building 3.2 as it is already included (and extended) there: https://github.com/OCamlPro/gnucobol/blob/gnucobol-3.2/configure.ac

Copy link
Author

@alexbodn alexbodn Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no configure.patch in my PR.
i'm patching configure.ac and autogenerating configure in the upstream part of the patch.

This file was deleted.

24 changes: 0 additions & 24 deletions packages/gnucobol/libcob-common.c.patch
View file
Open in desktop

This file was deleted.

11 changes: 0 additions & 11 deletions packages/gnucobol/libcob-mlio.c.patch
View file
Open in desktop
Copy link

@GitMensch GitMensch Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one patch file to do the include in both places would be necessary if building from last year's 3.2 dist tarball; can be dropped for 3.3 (or a theoretical possible 3.2.1) where this is solved

Copy link
Author

@alexbodn alexbodn Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the missing includes have nothing to do with termux, hence they're part of the upstream patch.

This file was deleted.

13 changes: 13 additions & 0 deletions packages/gnucobol/rmdoc.in.patch
View file
Open in desktop
Copy link

@GitMensch GitMensch Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of this just touch the target files provided in the distribution tarball after patching libcob

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/Makefile.in b/Makefile.in
index 6986591..d8f1de0 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -491,7 +491,7 @@ top_build_prefix = @top_build_prefix@
top_builddir = @top_builddir@
top_srcdir = @top_srcdir@
include_HEADERS = libcob.h
-SUBDIRS = . lib libcob bin cobc config copy po extras doc tests
+SUBDIRS = . lib libcob bin cobc config copy po extras
BUILT_SOURCES = tarstamp.h
ACLOCAL_AMFLAGS = -I m4 --install
# CLEANFILES = $(bin_SCRIPTS)
16 changes: 16 additions & 0 deletions packages/gnucobol/tmploc.patch
View file
Open in desktop
Copy link

@GitMensch GitMensch Jul 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A patch like this was also included in the old package, while the new name is better I do question if this is really needed - because this part is only applied if none of TMDIR, TMP, TEMP are set - and I think this is already the case, no?

Copy link
Author

@alexbodn alexbodn Jul 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the patch does apply, there is a "/tmp" literal somewhere.
anyway, if you set the catch all /tmp, i'm happy it could be supported in termux too.

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
diff --git a/libcob/common.c b/libcob/common.c
--- a/libcob/common.c
+++ b/libcob/common.c
@@ -5973,9 +5974,9 @@ cob_gettmpdir (void)
#else
&& (tmpdir = check_valid_env_tmpdir ("TMP")) == NULL
&& (tmpdir = check_valid_env_tmpdir ("TEMP")) == NULL) {
- if (!check_valid_dir ("/tmp")) {
- tmp = cob_fast_malloc (5U);
- strcpy (tmp, "/tmp");
+ if (!check_valid_dir ("@TERMUX_PREFIX@/tmp")) {
+ tmp = cob_fast_malloc (strlen("@TERMUX_PREFIX@") + 5U);
+ strcpy (tmp, "@TERMUX_PREFIX@/tmp");
Copy link
Member

@TomJo2000 TomJo2000 Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a pitty.
if upstream gives this fallback option, why not on termux too?
it's definitely and legitimately termux related.

Let's move the discussion of the /tmp patch to the patch in question.
I think this is good, and should be kept.
This is straight forward case of fixing a hardcoded path, it just so happens that $TMPDIR already does this job.

tmpdir = tmp;
}
#endif