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.
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
Remove remaining dependencies of bootstrap on gettext (AC_LIB_RPATH etc.) #34152
Comments
New commits:
|
Commit: |
Author: Matthias Koeppe |
This comment has been minimized.
This comment has been minimized.
Branch pushed to git repo; I updated commit sha1. New commits:
|
This comment has been minimized.
This comment has been minimized.
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. |
comment:8
it seems that "copying" is meant to be done from an installed |
comment:9
There's no such thing as an "installed" gnulib. |
comment:10
The workflow with gnulib is documented in https://www.gnu.org/software/gnulib/manual/html_node/Invoking-gnulib_002dtool.html |
comment:11
Replying to @dimpase:
from https://github.com/coreutils/gnulib/blob/master/top/gitsub.sh#L36: |
comment:12
Replying to @mkoeppe:
there certainly is an executable/script called |
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
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:
The quote you attribute to
The docs don't even discuss manual vendoring of files, I think. |
comment:17
Replying to @dimpase:
Yes, sorry, wrong link. |
comment:18
Replying to @dimpase:
Please, Dima, the whole manual talks about nothing else. |
comment:19
Replying to @mkoeppe:
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:
It's called the "initial import". https://www.gnu.org/software/gnulib/manual/html_node/Initial-import.html |
comment:21
Replying to @mkoeppe:
This manual is older than
|
comment:22
Replying to @dimpase:
It's a new option of how to work with gnulib, but it offers no benefits for us. |
comment:291
No, you are twisting my words, please stop. I never conducted a study of this (and I trust Bruno did, one way or another, arriving to the conclusion opposite). I also wrote there, and got a confirmation from a Gentoo developer, that "code |
comment:292
Replying to @dimpase:
In the previous message, https://lists.gnu.org/archive/html/bug-gnulib/2022-08/msg00102.html, Bruno writes this:
He is confirming what the manual is saying, disputing your comment:72 |
comment:293
This is fun. I suggest that we stop the discussion and just agree to abide by the vote on sage-devel, which is still ongoing. |
comment:294
+1, with the recommendation for everyone interested to read the whole thread in https://lists.gnu.org/archive/html/bug-gnulib/2022-08/msg00089.html |
comment:295
Replying to @kwankyu:
This only shows that asking people to vote on a proposal that was constructed by few other people without letting them to debate and improve the proposal first is a bad habit. We already experienced similar "vote don't discuss" procedure by the past, including the "code of conduct" episode. If a better proposal emerges, i do not see in the name of what it could not be chosen over the previous ones. |
comment:296
Replying to @sagetrac-tmonteil:
We would be choked if we start a discussion on the procedure of voting that is likely to lead to a voting :) If you want the discussion anyway, please open it on sage-devel. |
comment:297
Replying to @sagetrac-tmonteil:
This problem must urgently be resolved: presently, you can have a valid setup according to the documentation and yet be unable to build/configure the last couple of sagemath beta releases. As far as I could see, a valid solution could have been "gettext-devel is a prereq for sagemath" (a really ugly one, though) but the solutions on the ticket at the moment would work too. I think the priority is to get something merged that resolves the acute problem. Once we have a somewhat workable solution in place that at least allows people to build sage, we can look if the solution, like any piece of sage, can be improved. Who knows, perhaps a better solution even gets merged before a release is actually made! But we cannot really wait for that now. |
comment:298
Replying to @sagetrac-tmonteil:
Your suggestion has some merits but there is no one size fits all for all cases. Problem: you have people arguing over two possible solution. You invite more sage developers in and now you have people arguing over 8 different solutions (Ok, I am exaggerating but I really see that kind of things as a real possibility). I'll take responsibility for calling the vote on this particular occasion. I was invited on the ticket but it became clear that I wouldn't sway one side over the other and I had nothing much more to offer to the ticket. May be calling someone else would have been better in this case. |
comment:299
Not a large participation but of those who voted, the hybrid approach was clearly prefered.
|
comment:300
I reviewed Dima's script while transforming it to the |
Reviewer: ..., Matthias Koeppe |
This comment has been minimized.
This comment has been minimized.
Changed reviewer from ..., Matthias Koeppe to ..., Matthias Koeppe, Kwankyu Lee |
comment:302
It looks good to me, except those m4 files of external origin. But we decided to accept them. |
comment:303
Replying to @kwankyu:
If you put it like that, it's almost an argument for checking them in: by keeping them out of the tree, they'd be invisible for review and yet the files are used to build sagemath. When we put them into the repo at least it's clear for all to see those particular pieces of code that we depend on. We have of course plenty of dependencies that we don't put through our usual review process ... Anyway, thanks for getting the review process for this on track! I'm happy to see this problem is being resolved now. If people care enough to implement a nicer solution they can do so. |
comment:304
Replying to @nbruin:
Yes, this is an important point, and it matches an earlier discussion that we had on sage-devel regarding monorepo vs. multirepo strategies (in the context of the Sage library, not the Sage distribution): https://groups.google.com/g/sage-devel/c/AaKxoNQAWMg/m/uY1rW5n0BQAJ Related: https://groups.google.com/g/sage-devel/c/JBFbtUNhqNU/m/KuySNvTuBAAJ |
comment:305
Let's finally get this ticket in please. |
Changed reviewer from ..., Matthias Koeppe, Kwankyu Lee to François Bissey, Dima Pasechnik, Matthias Koeppe, Kwankyu Lee |
comment:307
Thanks! |
Changed branch from u/mkoeppe/config/gnulib to |
Changed reviewer from François Bissey, Dima Pasechnik, Matthias Koeppe, Kwankyu Lee to François Bissey, Dima Pasechnik, Matthias Koeppe, Kwankyu Lee |
Changed commit from |
Background:
./bootstrap
needs about 112 KB of m4 and shell code fromgnulib
: https://www.gnu.org/software/gnulibBefore #29549, these files were copied from a system-wide installation of
gettext
(which usesgnulib
): https://www.gnu.org/software/gettextAs opposed to
gettext
,gnulib
does not make releases, and many systems don't packagegnulib
at all. Thus one cannot uniformly rely ongnulib
being already installed. This essentially leaves two options for usinggnulib
in Sage - copy said 112K into the Sage git tree, or providegnulib
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 bybootstrap
).Who needs to run
bootstrap
and where:tox -e docker-...
and on GH Actions, in the Docker container for the tested platform.Who does not need to run
bootstrap
:configure
SPKG.History: #29549 intended to remove the dependency of
bootstrap
ongettext
. #29549 made it possible to runbootstrap
on themanylinux
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 whichgettext
is missing completely, as reported in https://groups.google.com/g/sage-release/c/FezzF5Q7Wt4/m/xyxVTaYgBAAJThis ticket:
Here we complete the task and remove
gettext
from variousdistros/
files etc.Two approaches have been proposed:
mkoeppe ("copy files") : Branch u/mkoeppe/fix_broken_bootstrap__ac_lib_rpath_etc__ @ 4011cc4608
config/
andm4/
, total 112 KB, have been imported by usinggnulib-tool import iconv
and committed to the branch.m4/ax_*.m4
from autoconf-archive. Different from our practice with SPKGs because the files are needed to generate theconfigure
script.gnulib-tool import
, remove unneeded files (that's where bootstrap: Clean up use of gettextize #29549 went wrong), commit the changes. (Updates are expected to be necessary only very rarely.)dimpase ("Sage pseudo-package"): sagemath/sagetrac-mirror@develop...u/dimpase/config/gnulib
gnulib
checkout is 250 MB).git
installed) dependency ongit
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).bootstrap
file to change the commit hash, verify that the list of files removed bymake bootstrap-clean
is still correct, commit this changebootstrap
operate a multi-megabyte git clone (on the 1st install and when there's a change of the gnulib commit hash) in theupstream/
directory increases complexity for no benefit."hybrid ("copy files, spkg-src script for maintenance"): Branch u/mkoeppe/config/gnulib
spkg-src
script (adapted from dimpase's branch)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
The text was updated successfully, but these errors were encountered: