sagemath/sage

Remove remaining dependencies of bootstrap on gettext (AC_LIB_RPATH etc.)

mkoeppe opened this issue · 338 comments

Background: ./bootstrap needs about 112 KB of m4 and shell code from gnulib: https://www.gnu.org/software/gnulib
Before #29549, these files were copied from a system-wide installation of gettext (which uses gnulib): https://www.gnu.org/software/gettext

As opposed to gettext, gnulib does not make releases, and many systems don't package gnulib at all. Thus one cannot uniformly rely on gnulib being already installed. This essentially leaves two options for using gnulib in Sage - copy said 112K into the Sage git tree, or provide gnulib as a Sage pseudo-package (pseudo, due to the chicken vs egg issue: one cannot just make it an ordinary spkg - it needs to be used by bootstrap).

Who needs to run bootstrap and where:

  • Any developer who switches to a branch that makes an SPKG upgrade or other changes to the build system, on their development machine.
  • Our portability testsuite, run locally using tox -e docker-... and on GH Actions, in the Docker container for the tested platform.

Who does not need to run bootstrap:

  • Users who only build released versions from tarballs - because they come pre-bootstrapped
  • Users who only build released versions from a git clone - because Sage falls back to downloading a tarball of the configure SPKG.

History: #29549 intended to remove the dependency of bootstrap on gettext. #29549 made it possible to run bootstrap on the manylinux platforms needed by cibuildwheel (#33800).

The branch of #29549 followed "copy files" approach. However, some files that gnulib-tool added were left out from the commit, so it breaks on machines on which gettext is missing completely, as reported in https://groups.google.com/g/sage-release/c/FezzF5Q7Wt4/m/xyxVTaYgBAAJ


This ticket:
Here we complete the task and remove gettext from various distros/ files etc.

Two approaches have been proposed:

mkoeppe ("copy files") : Branch u/mkoeppe/fix_broken_bootstrap__ac_lib_rpath_etc__ @ 4011cc4608

  • Follows approach 3 of the Gnulib manual section 'Integration with Version Control Systems'.
  • 5 files, in config/ and m4/, total 112 KB, have been imported by using gnulib-tool import iconv and committed to the branch.
  • Matches our practice with imported files m4/ax_*.m4 from autoconf-archive. Different from our practice with SPKGs because the files are needed to generate the configure script.
  • Introduces no new dependency.
  • Burden on a developer who authors an update ticket for these files: Obtain gnulib by cloning the repo, then re-run gnulib-tool import, remove unneeded files (that's where #29549 went wrong), commit the changes. (Updates are expected to be necessary only very rarely.)
  • Burden on users and other developers: None.
  • dimpase's criticism of this approach: "gnulib's code is not Sage's code, it has no place in Sage git tree; this way, updating the imported gnulib files is tricky and error-prone by design".

dimpase ("Sage pseudo-package"): sagemath/sagetrac-mirror@develop...u/dimpase/config/gnulib

  • Follows approach 2(C) of the Gnulib manual section 'Integration with Version Control Systems', but instead of using Gnulib's Programs for developing in Git checkouts, implements its own script (making checkout much faster, as full gnulib checkout is 250 MB).
  • Introduces a new (although one can't really do Sage development without git installed) dependency on git and on availability of internet connection at the first run of ./bootstrap in a tree, to pull 83 MB (fair to say that internet is needed for Sage development a lot, anyway).
  • Burden on a developer who authors an update ticket for these files: Edit the bootstrap file to change the commit hash, verify that the list of files removed by make bootstrap-clean is still correct, commit this change
  • Burden on other users: a pseudo-package needs to be installed once (and updated, automatically)
  • mkoeppe's criticism of this approach: "Having bootstrap operate a multi-megabyte git clone (on the 1st install and when there's a change of the gnulib commit hash) in the upstream/ directory increases complexity for no benefit."

hybrid ("copy files, spkg-src script for maintenance"): Branch u/mkoeppe/config/gnulib

  • Resolves the criticism on the "copy files" approach: "updating the imported gnulib files" is now assisted by the spkg-src script (adapted from dimpase's branch)
  • Resolves the criticism on the "Sage pseudo-package" approach: "operate a multi-megabyte git clone (on the 1st install and when there's a change of the gnulib commit hash)" is only done by authors of tickets that update gnulib, not by general users of bootstrap.

Voting took place at https://groups.google.com/g/sage-devel/c/E2qstbWXC7g


To test: For example tox -r -e local-homebrew-macos-minimal -- config.status

CC: @culler @dimpase @kiwifb @isuruf @nbruin

Component: build: configure

Author: Matthias Koeppe, Dima Pasechnik

Branch: 8bf9d97

Reviewer: François Bissey, Dima Pasechnik, Matthias Koeppe, Kwankyu Lee

Issue created by migration from https://trac.sagemath.org/ticket/34152

New commits:

65638b4m4/: Add missing dependencies of iconv.m4

Commit: 65638b4

Author: Matthias Koeppe

Description changed:

--- 
+++ 
@@ -1,2 +1,3 @@
 Broken in #29549, because I forgot to add some files.
 
+Reported in https://groups.google.com/g/sage-release/c/FezzF5Q7Wt4/m/xyxVTaYgBAAJ

Branch pushed to git repo; I updated commit sha1. New commits:

4011cc4.homebrew-build-env, build/pkgs/{_prereq,_bootstrap}/distros: Remove gettext

Changed commit from 65638b4 to 4011cc4

Description changed:

--- 
+++ 
@@ -1,3 +1,5 @@
 Broken in #29549, because I forgot to add some files.
 
+Also, `gettext` is no longer a bootstrapping prerequisite, so we remove it from various `distros/` files etc.
+
 Reported in https://groups.google.com/g/sage-release/c/FezzF5Q7Wt4/m/xyxVTaYgBAAJ
comment:6

vendoring more and more of gnulib...

should we instead install,on demand, gnulib itself?

comment:7

This is how Gnulib is designed to be used.
"Its components are intended to be shared at the source level, rather than being a library that gets built, installed, and linked against. Thus, there is no distribution tarball; the idea is to copy files from Gnulib into your own source tree."
https://www.gnu.org/software/gnulib/

comment:8

it seems that "copying" is meant to be done from an installed gnulib version.
That's how I read https://www.mail-archive.com/bug-gnulib@gnu.org/msg36476.html
where a way to do this is discussed.
gnulib has a script to facilitate such an installation: https://github.com/coreutils/gnulib/blob/master/top/gitsub.sh

comment:9

There's no such thing as an "installed" gnulib.

comment:11

Replying to @dimpase:

gnulib has a script to facilitate such an installation: https://github.com/coreutils/gnulib/blob/master/top/gitsub.sh

from https://github.com/coreutils/gnulib/blob/master/top/gitsub.sh#L36:
"The package maintainer may choose to use or not use git submodules." - I chose to not use git submodules.

comment:12

Replying to @mkoeppe:

There's no such thing as an "installed" gnulib.

there certainly is an executable/script called gnulib-tool - which is certainly a part of installed gnulib

comment:13

"gnulib-tool is not installed in a standard directory that is contained in the PATH variable. It needs to be run directly in the directory that contains the Gnulib source code." https://www.gnu.org/software/gnulib/manual/html_node/Invoking-gnulib_002dtool.html

comment:14

gnulib-tool is not installed in a standard directory that is contained in the PATH variable

OK, fine, but its directory has to appear somehow

comment:15

Yes, a developer who wants to update the portions of Gnulib that have been copied into the source tree will use a git clone of gnulib and invoke gnulib-tool from there.

comment:16

Replying to @mkoeppe:

Replying to @dimpase:

gnulib has a script to facilitate such an installation: https://github.com/coreutils/gnulib/blob/master/top/gitsub.sh

from https://www.gnu.org/software/gnulib/manual/html_node/Invoking-gnulib_002dtool.html:
"The package maintainer may choose to use or not use git submodules." - I chose to not use git submodules.

The quote you attribute to Invoking-gnulib_002dtool.html is actually from gitsub.sh.

gitsub.sh can do subcheckouts rather than submodules.

The docs don't even discuss manual vendoring of files, I think.

comment:17

Replying to @dimpase:

Replying to @mkoeppe:

Replying to @dimpase:

gnulib has a script to facilitate such an installation: https://github.com/coreutils/gnulib/blob/master/top/gitsub.sh

from https://www.gnu.org/software/gnulib/manual/html_node/Invoking-gnulib_002dtool.html:
"The package maintainer may choose to use or not use git submodules." - I chose to not use git submodules.

The quote you attribute to Invoking-gnulib_002dtool.html is actually from gitsub.sh.

Yes, sorry, wrong link.
Fixed now: I meant to paste https://github.com/coreutils/gnulib/blob/master/top/gitsub.sh#L36

comment:18

Replying to @dimpase:

The docs don't even discuss manual vendoring of files, I think.

Please, Dima, the whole manual talks about nothing else.

comment:19

Replying to @mkoeppe:

Yes, a developer who wants to update the portions of Gnulib that have been copied into the source tree will use a git clone of gnulib and invoke gnulib-tool from there.

but not manually - rather, as a part of pre-configuring a project. Checking in our repo huge chunks of gnulib is suboptimal.

comment:20

Replying to @dimpase:

Replying to @mkoeppe:

Yes, a developer who wants to update the portions of Gnulib that have been copied into the source tree will use a git clone of gnulib and invoke gnulib-tool from there.

but not manually

It's called the "initial import". https://www.gnu.org/software/gnulib/manual/html_node/Initial-import.html
This is what I did in #29549 (except that I forgot to git add some files).

comment:21

Replying to @mkoeppe:

Replying to @dimpase:

The docs don't even discuss manual vendoring of files, I think.

Please, Dima, the whole manual talks about nothing else.

This manual is older than git, you know, no wonder it won't mention it.

gitsub.sh is for the modern way of doing things, and that's what gnulib maintainer uses.

comment:22

Replying to @dimpase:

gitsub.sh is for the modern way of doing things, and that's what gnulib maintainer uses.

It's a new option of how to work with gnulib, but it offers no benefits for us.

comment:23

Replying to @dimpase:

This manual is older than git, you know, no wonder it won't mention it.

Use with git is documented in https://www.gnu.org/software/gnulib/manual/html_node/VCS-Issues.html; it describes several approaches.

comment:24

Replying to @dimpase:

Checking in our repo huge chunks of gnulib is suboptimal.

#29549 added 2 files, the current ticket is adding 4 more.

comment:25

yes, about 5000 loc, likely to cause trouble if not updated on time.

downstreams wouldn't be happy, maintenance is problematic.

why vendor if it can be avoided?

comment:26

Once again, this is the standard way to use gnulib.

comment:27

Replying to @dimpase:

why vendor if it can be avoided?

Because the alternative (the gitsub.sh -- which is to be copied into the source tree by the way) introduces a ridiculous amount of complexity.

comment:28

How about just requiring gnulib-tool ? macOS purists may have to jump an extra hoop, but it's very minor.

Changed commit from 4011cc4 to 02ea3ca

comment:29

here is a way to use gnulib-tool instead.

Based on the original branch of this ticket.
By the way, I also noticed that gettext is still mentioned in a lot of tox-related and CI-related files.


New commits:

6679325Revert "m4/: Add missing dependencies of iconv.m4"
fcd5c6cdon't vendor gnulib files
02ea3cause gnulib-tool from ./bootstrap

Reviewer: Dima Pasechnik

comment:31

No, we cannot assume that gnulib-tool (= all of gnulib) is available in the system.

comment:32

... because comment:9

Changed commit from 02ea3ca to f5a9f50

Branch pushed to git repo; I updated commit sha1. New commits:

f5a9f50git-ignore everything installed by gnulib-tool
comment:34

Replying to @mkoeppe:

... because comment:9

all the systems I checked either have gnulib package (with a reasonably recent, complete, version of gnuilib), or none at all, and then
it can be installed (ok, ok, cloned from its repo, and PATH adjusted accordingly)

comment:36

Replying to @dimpase:

or none at all

exactly, that's the problem

and then
it can be installed (ok, ok, cloned from its repo, and PATH adjusted accordingly)

But we certainly do not want to force every developer who wants to run ./bootstrap to do that.

comment:37

Replying to @mkoeppe:

Replying to @dimpase:

or none at all

exactly, that's the problem

and then
it can be installed (ok, ok, cloned from its repo, and PATH adjusted accordingly)

But we certainly do not want to force every developer who wants to run ./bootstrap to do that.

only these who use macOS and only these who do not use anything but Xcode might have to do this. And among the latter only these who need to build ./configure

It's certainly not "every developer".

comment:38

Assuming that gnulib is installed in the system directly contradicts the intentions of upstream.

And neither homebrew nor conda have a gnulib package.

comment:39

Replying to @mkoeppe:

Assuming that gnulib is installed in the system directly contradicts the intentions of upstream.

one would expect the system maintainers to update reasonably often.
(indeed, e.g. on Debian stable and Fedora 34 gnulib is dated 2021; it appears that https://repology.org/project/gnulib/versions is not complete/up to date, e.g. is doesn't show Fedora packages).

No need for us to do this maintainance job.

And neither homebrew nor conda have a gnulib package.

for Homebrew we can actually make one; not sure how hard it's for conda

comment:40

Let me try to explain it again:

Upstream states clearly that gnulib is not to be installed.
And indeed, it does not have a make install target.

When Debian/Fedora/... are shipping gnulib, it is likely for the purpose of implementing their policy when building packages -- i.e., updating the vendored portions of gnulib of a package before starting the build.

comment:41

Replying to @mkoeppe:

Let me try to explain it again:

Upstream states clearly that gnulib is not to be installed.
And indeed, it does not have a make install target.

When !Debian/Fedora/... are shipping gnulib, it is likely for the purpose of implementing their policy when building packages -- i.e., updating the vendored portions of gnulib of a package before starting the build.

My reading is that gnulib's upstream doesn't want gnulib to be vendored, either, as it leads to using outdated gnulib.
The upstream wants a fresh version to be used (well, this is understandable). That's why they promote gitsub.sh.

comment:42

Replying to @dimpase:

My reading is that gnulib's upstream doesn't want gnulib to be vendored

My branch does not "vendor" gnulib. It has imported the needed gnulib modules into our source tree. This is the standard documented usage.

comment:43

Replying to @dimpase:

That's why they promote gitsub.sh.

They don't "promote" it. The entire manual only mentions the existence of this file. see https://www.gnu.org/software/gnulib/manual/gnulib.html

comment:44

Replying to @dimpase:

The upstream wants a fresh version to be used

But not "fresh" as in "whatever is in upstream gnulib HEAD" whenever someone runs ./bootstrap.

Updating the imported gnulib modules is something that the maintainer of a package does explicitly. For example, gettext does use gitsub.sh (in "submodule" mode): updating is done by a maintainer - that's a commit.

comment:45

the relationship between gettext and gnulib is akin to the relationship between individual strands of pasta in a spaghetti bowl :-)

comment:46

Replying to @mkoeppe:

Replying to @dimpase:

The upstream wants a fresh version to be used

But not "fresh" as in "whatever is in upstream gnulib HEAD" whenever someone runs ./bootstrap.

but, how fresh? Do they advice to age a version in a wine cask?

Updating the imported gnulib modules is something that the maintainer of a package does explicitly. For example, gettext does use gitsub.sh (in "submodule" mode): updating is done by a maintainer - that's a commit.

Aren't they maintained by the same people?

comment:47

Replying to @dimpase:

Replying to @mkoeppe:

Replying to @dimpase:

The upstream wants a fresh version to be used

But not "fresh" as in "whatever is in upstream gnulib HEAD" whenever someone runs ./bootstrap.

but, how fresh?

Update, test, commit, as usual

comment:48

the gettext worked for us without committing any files/modules from it, at all. I don't see how using gnulib instead is going to break anything.

comment:49

Before #29549, we were getting our files at ./bootstrap time from a system installation of gettextize. gettextize has the files that we need because gettext has imported these gnulib modules - see for example https://github.com/autotools-mirror/gettext/blob/master/autogen.sh#L168 for iconv.

comment:50

#29549 followed the advice from Bruno Haible that you relayed in #27823 comment:52 (#29549 comment:3).
Except for the mistake of forgetting git add for some of the files, which is entirely my own.

New commits:

65638b4m4/: Add missing dependencies of iconv.m4
4011cc4.homebrew-build-env, build/pkgs/{_prereq,_bootstrap}/distros: Remove gettext

Changed commit from f5a9f50 to 4011cc4

comment:52

I don't see how advice from Haible says that importing means copying+committing, rather than calling gnulib-tool every time autoconf has to be run.

comment:53

Replying to @dimpase:

I don't see how advice from Haible says that importing means copying+committing

That message only says, don't try to get it from gettext, get it from gnulib instead.

comment:54

I responded to your question in comment:48.

comment:55

I don't think you responded to my question in comment:48

comment:56

sorry, I see committing a bunch of files from another project as a regression (unneeded vendoring).

comment:57

Replying to @dimpase:

I don't think you responded to my question in comment:48

Try rephrasing your question then please

comment:58

Replying to @dimpase:

sorry, I see committing a bunch of files from another project as a regression (unneeded vendoring).

comment:42.

comment:59

I think Matthias is using gnulib as intended. Making it maintainable is possibly the tricky bit. Had to deal with gnulib before on AIX :( the advantage of using gettext in this case is to rely on a third party to do the maintenance. And then you have to make sure you are up to date yourself.

As to adding bunch of files from another project, yes it is trading them against the gettext dependency as far as I understand. So potentially a tarball with stuff you don't care about.

Dima, do you have a good stated technical opposition to gnulib rather than a philosophical one?

comment:60

I don't have opposition to gnulib, I rather want have it installed and used by ./bootstrap via gnulib-tool.

comment:61

Replying to @dimpase:

I don't have opposition to gnulib, I rather want have it installed and used by ./bootstrap via gnulib-tool.

I see, then we'll go round and round. Do I want to have to install gnulib to bootstrap?

fbissey@tarazed ~ $ eix gnulib
* dev-libs/gnulib
     Available versions:  ~*2019.03.17.09.24.57 ~*2022.02.12.16.27.05 ~*2022.05.26.07.24.56 **9999-r1*l {doc}
     Homepage:            https://www.gnu.org/software/gnulib
     Description:         Gnulib is a library of common routines intended to be shared at the source level

hum stuff that has all the markers for "install at your own risks, you have been warned" and which force to do things that are equivalent to signing a contract saying you understand the risks before you can install. Looking at the package itself it is "grab a tarball and manually copy the files in a potentially arbitrary location". No, I am with Matthias on that one, this is in a different class than autotools and libtools or cmake. You redistribute the bits you need. Having the tools to maintain it in the same package is a must though.

comment:62

"bits" are thousands of lines, in about 10 files. To me it screams that it should be installed and copied as needed, not copied once and committed in our tree.

I think gnulib is in such a weird state due to GNU folks having badly failed QA on the gnulib/gettext stuff.

comment:63

Replying to @dimpase:

I think gnulib is in such a weird state due to GNU folks having badly failed QA on the gnulib/gettext stuff.

Possibly :) that has never stopped anyone - and I am talking in a professional capacity as someone who helped people install real stinky software on "supercomputers" and computers not really super. Sometimes, I turn to my fellow researcher or a PhD student and go "we are not going to do that one sorry, it stinks because it is real dead. Please reconsider". Not so long ago sagemath was in the stinky pile, it still stinks a bit but not that bad :), using gnulib as Matthias wants will not make it smell much more. At the very least, gnulib and sagemath are very much alive and that counts for a lot.

comment:64

IMHO my branch for the issue, i.e. u/dimpase/config/gnulib, is less stinky.

Matthias already caused a bit of havoc in 9.7.beta5 by only keeping in m4/ some of the files installed by gnulib-tool --import iconv and not all (a bug only visible on a box without gettext installed :-(). And still u/mkoeppe/fix_broken_bootstrap__ac_lib_rpath_etc__ doesn't add them all.


New commits:

6679325Revert "m4/: Add missing dependencies of iconv.m4"
fcd5c6cdon't vendor gnulib files
02ea3cause gnulib-tool from ./bootstrap
f5a9f50git-ignore everything installed by gnulib-tool

Changed commit from 4011cc4 to f5a9f50

comment:65

advice to install gnutls is missing in this branch, but that's minor, easy to fix.

comment:66

Replying to @kiwifb:

Replying to @dimpase:

I don't have opposition to gnulib, I rather want have it installed and used by ./bootstrap via gnulib-tool.

I see, then we'll go round and round. Do I want to have to install gnulib to bootstrap?

fbissey@tarazed ~ $ eix gnulib
* dev-libs/gnulib
     Available versions:  ~*2019.03.17.09.24.57 ~*2022.02.12.16.27.05 ~*2022.05.26.07.24.56 **9999-r1*l {doc}
     Homepage:            https://www.gnu.org/software/gnulib
     Description:         Gnulib is a library of common routines intended to be shared at the source level

on gentoo I had to do the "missing keyword" fix for gnulib, in order to install it.
Not sure it's a bug or feature.

comment:67

It is a feature. This is a way to get you to sign off that you know what you are doing. You have to be really explicit that you want to install it.

comment:68

Replying to @mkoeppe:

Replying to @dimpase:

This manual is older than git, you know, no wonder it won't mention it.

Use with git is documented in https://www.gnu.org/software/gnulib/manual/html_node/VCS-Issues.html; it describes several approaches.

Dima, let me highlight this link again. It documents 3 approaches how to use gnulib with version control systems. Do you see which of the 3 corresponds to my branch?

Changed reviewer from Dima Pasechnik to none

comment:69

After all these years trying to unvendor as much as possible, all of a sudden you reverse the course. I don't get it.

Changed commit from f5a9f50 to none

Changed branch from u/dimpase/config/gnulib to none

comment:70

You keep using this word "vendoring" here. This is not what's happening. My branch is using gnulib as documented.

You may find https://lists.gnu.org/archive/html/bug-gnulib/2008-11/msg00083.html helpful.

New commits:

65638b4m4/: Add missing dependencies of iconv.m4
4011cc4.homebrew-build-env, build/pkgs/{_prereq,_bootstrap}/distros: Remove gettext

Commit: 4011cc4

comment:72

I consider the gnulib manual outdated. OK, it's not vendoring, it's an SCO (Special Copying Operation) :P

comment:73

Replying to @dimpase:

I consider the gnulib manual outdated.

Try taking that to bug-gnulib.

comment:74

Pro tip: Use the subject "Gnulib considered harmful"

Description changed:

--- 
+++ 
@@ -1,5 +1,9 @@
-Broken in #29549, because I forgot to add some files.
+#29549 intended to remove the dependency of `bootstrap` on gettext. #29549 made it possible to run `bootstrap` on the `manylinux` platforms needed by cibuildwheel (#33800).
 
-Also, `gettext` is no longer a bootstrapping prerequisite, so we remove it from various `distros/` files etc.
+However, I forgot to add some files that `gnulib-tool` added, so it breaks on machines on which `gettext` is missing completely, as reported in https://groups.google.com/g/sage-release/c/FezzF5Q7Wt4/m/xyxVTaYgBAAJ
 
-Reported in https://groups.google.com/g/sage-release/c/FezzF5Q7Wt4/m/xyxVTaYgBAAJ
+Here we complete the task and remove `gettext` from various `distros/` files etc.
+
+To test: For example `tox -r -e local-homebrew-macos-minimal -- config.status`
+
+
comment:76

Adding those four files to the m4 directory fixed all of my problems with the bootstrap script, including the missing .dylib extension on LIBSINGULAR_PATH. I am now getting a working macOS build with no issues.

It seems completely clear to me that this is the correct approach. A project distributes an m4 directory containing the exact gnulib .m4 files that are needed to make its build work. By that I mean the exact versions of those files. The gnulib repository changes almost daily and they do not do releases. So this seems like the only way to ensure a working build. And it is also what gnulib says to do.

comment:77

Replying to @culler:

Adding those four files to the m4 directory fixed all of my problems with the bootstrap script, including the missing .dylib extension on LIBSINGULAR_PATH. I am now getting a working macOS build with no issues.

It seems completely clear to me that this is the correct approach. A project distributes an m4 directory containing the exact gnulib .m4 files that are needed to make its build work. By that I mean the exact versions of those files. The gnulib repository changes almost daily and they do not do releases. So this seems like the only way to ensure a working build. And it is also what gnulib says to do.

The only way?
If one wants to follow the gnulib updates then one can use git submodule or a similar solution; and this is mentioned in gnulib docs.
Totally no need to follow the 30+ years old way of copying files by hand.

comment:78

Gnulib provides gnulib-tool for the copying, so copying is not done "by hand".

comment:79

sure - but updating is done by hand; one needs to update gnulib on the box, and launch gnulib-tool

comment:80

Yes, the author of an upgrade ticket for these files will have to run gnulibtool.

comment:81

That's approach 3 of the three approaches explained in the manual (https://www.gnu.org/software/gnulib/manual/html_node/VCS-Issues.html).

comment:82

so you chose the most labour-intensive approach, resulting in extra maintenance to be done.

comment:83

No.

In approach 2 of the three approaches, in the variant with git submodule, the author of an upgrade ticket will have to run git submodule update --remote and then commit the change.

For the author of an upgrade ticket for gnulib, it's the same amount of labor as approach 3: One command and one commit.

The drawbacks of this approach:

  • it exposes all developers (not just an author of a gnulib upgrade ticket) to the complexity of git submodules.
  • a source tarball of Sage will have to contain the full copy of gnulib (a copy of the submodule). More "vendoring" ;)
comment:84

why do you insist on committing exact copies of files in gnulib into Sage tree? My approach does not do this.

comment:85

Replying to @dimpase:

why do you insist on committing exact copies of files in gnulib into Sage tree?

It is the documented way to use gnulib, specifically, approach 3.

The Gnulib manual, https://www.gnu.org/software/gnulib/manual/html_node/VCS-Issues.html, introduces the three approaches with this paragraph:

Here are the three different approaches in common use. Each has its place, and you should use whichever best suits your particular project and development methods.

The approach of your branch is neither of the three documented approaches. Upstream explicitly rejects this approach. It's reflected in: Gnulib not making releases and not having an "install" target (comment:9, comment:40); homebrew and conda not packaging it (comment:38); gentoo marking it as "source only" (comment:67); and the thread linked in comment:70.

comment:86

I think upstream has it wrong. Why do they insist on partial, or even complete, vendoring of their code? This only make sense if one wants to change it, but for 99% of projects it is not the case.