Skip to content
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

Bootstrap libltdl to fix libtool v2.4 + automake v1.17 #1900

Closed
wants to merge 2 commits into from

Conversation

kinkie
Copy link
Contributor

@kinkie kinkie commented Sep 15, 2024

Bootstrap libltdl to fix libtool v2.4 + automake v1.17 build

gmake[3]: Entering directory .../libltdl
.../cfgaux/missing: line 85: aclocal-1.16: command not found
gmake[3]: *** [Makefile:561: .././../libltdl/aclocal.m4]
Error 127

During bootstrap.sh run, libtoolize copies prepackaged
configure and Makefile.in files into our libltdl directory:

  • libltdl/configure from libtool v2.4 has aclocal version set
    to 1.16;
  • libltdl/Makefile.in from libtool v2.4 uses
    configure-set aclocal version to build aclocal.m4

Thus, libltdl/Makefile (generated from libltdl/Makefile.in
above) runs aclocal-1.16 if "make" needs to build
libltdl/aclocal.m4.

Normally, "make" does not need to build libltdl/aclocal.m4
because that file was created by libtoolize. However, libtool
v2.4 is packaged with (generated by packaging folks)
libltdl/Makefile.in that makes libltdl/aclocal.m4 target
dependent on files in libltld/../m4 directory. Squid does not
have that ./m4 directory, so "make" attempts to re-generate
libltdl/aclocal.m4. When it does, it uses aclocal-1.16.

Our bootstrap.sh generated new ./configure but preserved
copied libltdl/configure with its aclocal version set to
1.16. In other words, our bootstrap.sh did not bootstrap
libltdl sub-project. In build environments without
aclocal-1.16, Squid build failed.

Several solutions or workarounds have been tried or
considered:

  • Adjust bootstrap.sh to bootstrap libltdl (this change).
    2008 attempt to do that was reverted in commit bfd6b6a with
    "better to fix libtool installation" rationale. Another
    potential argument against this option is that packages
    should be bootstrapped by their distributors, not "users". We
    are not distributing libtool, but this is a gray area because
    we do distribute files that libtoolize creates. Finally,
    libtool itself does not provide a bootstrapping script and
    does not explicitly recommend bootstrapping in documentation.

  • "Fix libtool installation". We failed to find a good way to
    do that on MacOS (without building and installing newer
    libtool from sources).

  • Place m4 files where libtool v2.4 expects to find them.
    That change fixes MacOS builds that use automake v1.17, but
    breaks Gentoo builds because Gentoo libtool installs a buggy
    libltdl/Makefile.in that must be regenerated by automake
    before it can work. Fixing m4 files location prevents that
    regeneration.

We picked the first option despite its drawbacks because the
third option did not work on Gentoo, and asking Squid
developers to install libtool from sources (i.e. the second
option) felt like a greater evil.

This old problem was exposed by recently introduced CI MacOS
tests that started to fail when MacOS brew updated automake
package from v1.16 without the corresponding libtoolize
package changes.

Also work around what seems to be a libtool packaging bug
affecting MacOS/brew environments, including GitHub Actions
runners we use for CI:

libtool  (2.4.7) : glibtool libtool path :
/opt/homebrew/bin Bootstrapping glibtoolize:   error:
creating 'libltdl/configure.ac' ... failed glibtoolize:
error: creating 'libltdl/configure' ... failed glibtoolize
failed

That workaround will be removed after libtool package is
fixed.

Also removed a single-iteration "for dir" loop with several
stale hacks from bootstrap.sh: With only two directories to
bootstrap and with a directory-specific mkdir command, source
comments, and progress messages, it is best to unroll that
loop.


Backport of PR #1877

…ache#1877)

    gmake[3]: Entering directory .../libltdl
    .../cfgaux/missing: line 85: aclocal-1.16: command not found
    gmake[3]: *** [Makefile:561: .././../libltdl/aclocal.m4] Error 127

During bootstrap.sh run, libtoolize copies prepackaged configure and
Makefile.in files into our libltdl directory:

* libltdl/configure from libtool v2.4 has aclocal version set to 1.16;
* libltdl/Makefile.in from libtool v2.4 uses configure-set aclocal
  version to build aclocal.m4

Thus, libltdl/Makefile (generated from libltdl/Makefile.in above) runs
aclocal-1.16 if "make" needs to build libltdl/aclocal.m4.

Normally, "make" does not need to build libltdl/aclocal.m4 because that
file was created by libtoolize. However, libtool v2.4 is packaged with
(generated by packaging folks) libltdl/Makefile.in that makes
libltdl/aclocal.m4 target dependent on files in libltld/../m4 directory.
Squid does not have that ./m4 directory, so "make" attempts to
re-generate libltdl/aclocal.m4. When it does, it uses aclocal-1.16.

Our bootstrap.sh generated new ./configure but preserved copied
libltdl/configure with its aclocal version set to 1.16. In other words,
our bootstrap.sh did not bootstrap libltdl sub-project. In build
environments without aclocal-1.16, Squid build failed.

Several solutions or workarounds have been tried or considered:

* Adjust bootstrap.sh to bootstrap libltdl (this change). 2008 attempt
  to do that was reverted in commit bfd6b6a with "better to fix libtool
  installation" rationale. Another potential argument against this
  option is that packages should be bootstrapped by their distributors,
  not "users". We are not distributing libtool, but this is a gray area
  because we do distribute files that libtoolize creates. Finally,
  libtool itself does not provide a bootstrapping script and does not
  explicitly recommend bootstrapping in documentation.

* "Fix libtool installation". We failed to find a good way to do that on
  MacOS (without building and installing newer libtool from sources).

* Place m4 files where libtool v2.4 expects to find them. That change
  fixes MacOS builds that use automake v1.17, but breaks Gentoo builds
  because Gentoo libtool installs a buggy libltdl/Makefile.in that must
  be regenerated by automake before it can work. Fixing m4 files
  location prevents that regeneration.

We picked the first option despite its drawbacks because the third
option did not work on Gentoo, and asking Squid developers to install
libtool from sources (i.e. the second option) felt like a greater evil.

This old problem was exposed by recently introduced CI MacOS tests that
started to fail when MacOS brew updated automake package from v1.16
without the corresponding libtoolize package changes.

Also work around what seems to be a libtool packaging bug affecting
MacOS/brew environments, including GitHub Actions runners we use for CI:

    libtool  (2.4.7) : glibtool
    libtool path : /opt/homebrew/bin
    Bootstrapping
    glibtoolize:   error: creating 'libltdl/configure.ac' ... failed
    glibtoolize:   error: creating 'libltdl/configure' ... failed
    glibtoolize failed

That workaround will be removed after libtool package is fixed.

Also removed a single-iteration "for dir" loop with several stale hacks
from bootstrap.sh: With only two directories to bootstrap and with a
directory-specific mkdir command, source comments, and progress
messages, it is best to unroll that loop.
@squid-anubis squid-anubis added the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Sep 15, 2024
@rousskov
Copy link
Contributor

rousskov commented Sep 16, 2024

@kinkie, please note that PR title is malformed (i.e. has open parenthesis) and has unwanted (IMO) PR reference. PR description has very long lines. PR changes contain an out-of-scope MacOS CI test addition.

@kinkie kinkie changed the title Backport: bootstrap libltdl (PR #1877 Backport: bootstrap libltdl (PR #1877) Sep 16, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Sep 16, 2024

@kinkie, please note that PR title is malformed (i.e. has open parenthesis) and has unwanted (IMO) PR reference. PR description has very long lines. PR changes contain an out-of-scope MacOS CI test addition.

Both should be addressed now

@rousskov
Copy link
Contributor

please note that PR title is malformed (i.e. has open parenthesis) and has unwanted (IMO) PR reference. PR description has very long lines. PR changes contain an out-of-scope MacOS CI test addition.

Both should be addressed now

There are at least three problems: I noticed the corresponding label and added "PR description has very long lines" sentence after the original comment was emailed to you.

@kinkie
Copy link
Contributor Author

kinkie commented Sep 17, 2024

PR description has very long lines

It's very surprising, since the whole commit is a git cherry-pick.
Reformatting it

@squid-anubis squid-anubis removed the M-failed-description https://github.com/measurement-factory/anubis#pull-request-labels label Sep 17, 2024
@rousskov
Copy link
Contributor

It's very surprising, since the whole commit is a git cherry-pick.

I doubt that this PR branch commit (aa8f2dd) was a pure cherry-pick of a master/v7 commit -- the corresponding commit in master/v7 did not add MacOS tests. I suspect some manual editing was involved...

However, the commit message formatting in that PR branch commit is fine (the message was probably generated by git and not hand-edited). The (still present IMO) problems are with PR title and description, but I have given up asking for changes in metadata for backporting PRs.

@kinkie kinkie changed the title Backport: bootstrap libltdl (PR #1877) Bootstrap libltdl to fix libtool v2.4 + automake v1.17 build Sep 17, 2024
@kinkie kinkie changed the title Bootstrap libltdl to fix libtool v2.4 + automake v1.17 build v6: Bootstrap libltdl to fix libtool v2.4 + automake v1.17 Sep 17, 2024
@kinkie kinkie added the S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box label Sep 17, 2024
@rousskov
Copy link
Contributor

- Bootstrap libltdl to fix libtool v2.4 + automake v1.17 build
+ v6: Bootstrap libltdl to fix libtool v2.4 + automake v1.17

Are you sure you want v6: title prefix in a commit on a v6 branch? Your call (I do not even know what problems you are trying to solve), but it feels very redundant to me.

@rousskov rousskov added S-waiting-for-author author action is expected (and usually required) and removed S-waiting-for-reviewer ready for review: Set this when requesting a (re)review using GitHub PR Reviewers box labels Sep 18, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Sep 18, 2024

Are you sure you want v6: title prefix in a commit on a v6 branch? Your call (I do not even know what problems you are trying to solve), but it feels very redundant to me.

I was thinking to use 'Backport' but there is not enough space to fit within the character limit.
I can also remove the prefix

@kinkie kinkie changed the title v6: Bootstrap libltdl to fix libtool v2.4 + automake v1.17 Bootstrap libltdl to fix libtool v2.4 + automake v1.17 Sep 18, 2024
@rousskov
Copy link
Contributor

I would remove the prefix, but, again, I do not know what problems you are trying to solve by adding a prefix.

If you want to add backporting information and other details not present in the cherry-picked commit, I recommend using a commit message footer. You can find lots of examples in commit messages inside unofficial Factory branches like https://github.com/measurement-factory/squid/tree/bag51

squid-anubis pushed a commit that referenced this pull request Sep 25, 2024
Bootstrap libltdl to fix libtool v2.4 + automake v1.17 build

    gmake[3]: Entering directory .../libltdl
    .../cfgaux/missing: line 85: aclocal-1.16: command not found
    gmake[3]: *** [Makefile:561: .././../libltdl/aclocal.m4]
    Error 127

During bootstrap.sh run, libtoolize copies prepackaged
configure and Makefile.in files into our libltdl directory:

* libltdl/configure from libtool v2.4 has aclocal version set
  to 1.16;
* libltdl/Makefile.in from libtool v2.4 uses
  configure-set aclocal version to build aclocal.m4

Thus, libltdl/Makefile (generated from libltdl/Makefile.in
above) runs aclocal-1.16 if "make" needs to build
libltdl/aclocal.m4.

Normally, "make" does not need to build libltdl/aclocal.m4
because that file was created by libtoolize. However, libtool
v2.4 is packaged with (generated by packaging folks)
libltdl/Makefile.in that makes libltdl/aclocal.m4 target
dependent on files in libltld/../m4 directory. Squid does not
have that ./m4 directory, so "make" attempts to re-generate
libltdl/aclocal.m4. When it does, it uses aclocal-1.16.

Our bootstrap.sh generated new ./configure but preserved
copied libltdl/configure with its aclocal version set to
1.16. In other words, our bootstrap.sh did not bootstrap
libltdl sub-project. In build environments without
aclocal-1.16, Squid build failed.

Several solutions or workarounds have been tried or
considered:

* Adjust bootstrap.sh to bootstrap libltdl (this change).
  2008 attempt to do that was reverted in commit bfd6b6a with
  "better to fix libtool installation" rationale. Another
  potential argument against this option is that packages
  should be bootstrapped by their distributors, not "users". We
  are not distributing libtool, but this is a gray area because
  we do distribute files that libtoolize creates. Finally,
  libtool itself does not provide a bootstrapping script and
  does not explicitly recommend bootstrapping in documentation.

* "Fix libtool installation". We failed to find a good way to
  do that on MacOS (without building and installing newer
  libtool from sources).

* Place m4 files where libtool v2.4 expects to find them.
  That change fixes MacOS builds that use automake v1.17, but
  breaks Gentoo builds because Gentoo libtool installs a buggy
  libltdl/Makefile.in that must be regenerated by automake
  before it can work. Fixing m4 files location prevents that
  regeneration.

We picked the first option despite its drawbacks because the
third option did not work on Gentoo, and asking Squid
developers to install libtool from sources (i.e. the second
option) felt like a greater evil.

This old problem was exposed by recently introduced CI MacOS
tests that started to fail when MacOS brew updated automake
package from v1.16 without the corresponding libtoolize
package changes.

Also work around what seems to be a libtool packaging bug
affecting MacOS/brew environments, including GitHub Actions
runners we use for CI:

    libtool  (2.4.7) : glibtool libtool path :
    /opt/homebrew/bin Bootstrapping glibtoolize:   error:
    creating 'libltdl/configure.ac' ... failed glibtoolize:
    error: creating 'libltdl/configure' ... failed glibtoolize
    failed

That workaround will be removed after libtool package is
fixed.

Also removed a single-iteration "for dir" loop with several
stale hacks from bootstrap.sh: With only two directories to
bootstrap and with a directory-specific mkdir command, source
comments, and progress messages, it is best to unroll that
loop.
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Sep 25, 2024
squid-anubis pushed a commit that referenced this pull request Sep 27, 2024
Bootstrap libltdl to fix libtool v2.4 + automake v1.17 build

    gmake[3]: Entering directory .../libltdl
    .../cfgaux/missing: line 85: aclocal-1.16: command not found
    gmake[3]: *** [Makefile:561: .././../libltdl/aclocal.m4]
    Error 127

During bootstrap.sh run, libtoolize copies prepackaged
configure and Makefile.in files into our libltdl directory:

* libltdl/configure from libtool v2.4 has aclocal version set
  to 1.16;
* libltdl/Makefile.in from libtool v2.4 uses
  configure-set aclocal version to build aclocal.m4

Thus, libltdl/Makefile (generated from libltdl/Makefile.in
above) runs aclocal-1.16 if "make" needs to build
libltdl/aclocal.m4.

Normally, "make" does not need to build libltdl/aclocal.m4
because that file was created by libtoolize. However, libtool
v2.4 is packaged with (generated by packaging folks)
libltdl/Makefile.in that makes libltdl/aclocal.m4 target
dependent on files in libltld/../m4 directory. Squid does not
have that ./m4 directory, so "make" attempts to re-generate
libltdl/aclocal.m4. When it does, it uses aclocal-1.16.

Our bootstrap.sh generated new ./configure but preserved
copied libltdl/configure with its aclocal version set to
1.16. In other words, our bootstrap.sh did not bootstrap
libltdl sub-project. In build environments without
aclocal-1.16, Squid build failed.

Several solutions or workarounds have been tried or
considered:

* Adjust bootstrap.sh to bootstrap libltdl (this change).
  2008 attempt to do that was reverted in commit bfd6b6a with
  "better to fix libtool installation" rationale. Another
  potential argument against this option is that packages
  should be bootstrapped by their distributors, not "users". We
  are not distributing libtool, but this is a gray area because
  we do distribute files that libtoolize creates. Finally,
  libtool itself does not provide a bootstrapping script and
  does not explicitly recommend bootstrapping in documentation.

* "Fix libtool installation". We failed to find a good way to
  do that on MacOS (without building and installing newer
  libtool from sources).

* Place m4 files where libtool v2.4 expects to find them.
  That change fixes MacOS builds that use automake v1.17, but
  breaks Gentoo builds because Gentoo libtool installs a buggy
  libltdl/Makefile.in that must be regenerated by automake
  before it can work. Fixing m4 files location prevents that
  regeneration.

We picked the first option despite its drawbacks because the
third option did not work on Gentoo, and asking Squid
developers to install libtool from sources (i.e. the second
option) felt like a greater evil.

This old problem was exposed by recently introduced CI MacOS
tests that started to fail when MacOS brew updated automake
package from v1.16 without the corresponding libtoolize
package changes.

Also work around what seems to be a libtool packaging bug
affecting MacOS/brew environments, including GitHub Actions
runners we use for CI:

    libtool  (2.4.7) : glibtool libtool path :
    /opt/homebrew/bin Bootstrapping glibtoolize:   error:
    creating 'libltdl/configure.ac' ... failed glibtoolize:
    error: creating 'libltdl/configure' ... failed glibtoolize
    failed

That workaround will be removed after libtool package is
fixed.

Also removed a single-iteration "for dir" loop with several
stale hacks from bootstrap.sh: With only two directories to
bootstrap and with a directory-specific mkdir command, source
comments, and progress messages, it is best to unroll that
loop.
@kinkie
Copy link
Contributor Author

kinkie commented Sep 28, 2024

I would remove the prefix, but, again, I do not know what problems you are trying to solve by adding a prefix.

I aim to just highlight that it's a backport.
But I agree that once it's in, it's in.
Automatically backported PRs reference the original PR number, whcih I know you are not happy about.
I'll just merge, this is just useful to have in; it's clear enough

@kinkie kinkie added M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels and removed S-waiting-for-author author action is expected (and usually required) M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels labels Sep 28, 2024
@squid-anubis squid-anubis added the M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels label Sep 28, 2024
@rousskov
Copy link
Contributor

rousskov commented Sep 30, 2024

@eduard-bagdasaryan, please check why Anubis is not merging this v6 PR despite an all-green staged commit (c382818).

@rousskov
Copy link
Contributor

I aim to just highlight that it's a backport.

As you know, I recommend using a commit message footer for that.

P.S. To avoid a misunderstanding, I do not insist on any changes to this PR.

squid-anubis pushed a commit that referenced this pull request Sep 30, 2024
Bootstrap libltdl to fix libtool v2.4 + automake v1.17 build

    gmake[3]: Entering directory .../libltdl
    .../cfgaux/missing: line 85: aclocal-1.16: command not found
    gmake[3]: *** [Makefile:561: .././../libltdl/aclocal.m4]
    Error 127

During bootstrap.sh run, libtoolize copies prepackaged
configure and Makefile.in files into our libltdl directory:

* libltdl/configure from libtool v2.4 has aclocal version set
  to 1.16;
* libltdl/Makefile.in from libtool v2.4 uses
  configure-set aclocal version to build aclocal.m4

Thus, libltdl/Makefile (generated from libltdl/Makefile.in
above) runs aclocal-1.16 if "make" needs to build
libltdl/aclocal.m4.

Normally, "make" does not need to build libltdl/aclocal.m4
because that file was created by libtoolize. However, libtool
v2.4 is packaged with (generated by packaging folks)
libltdl/Makefile.in that makes libltdl/aclocal.m4 target
dependent on files in libltld/../m4 directory. Squid does not
have that ./m4 directory, so "make" attempts to re-generate
libltdl/aclocal.m4. When it does, it uses aclocal-1.16.

Our bootstrap.sh generated new ./configure but preserved
copied libltdl/configure with its aclocal version set to
1.16. In other words, our bootstrap.sh did not bootstrap
libltdl sub-project. In build environments without
aclocal-1.16, Squid build failed.

Several solutions or workarounds have been tried or
considered:

* Adjust bootstrap.sh to bootstrap libltdl (this change).
  2008 attempt to do that was reverted in commit bfd6b6a with
  "better to fix libtool installation" rationale. Another
  potential argument against this option is that packages
  should be bootstrapped by their distributors, not "users". We
  are not distributing libtool, but this is a gray area because
  we do distribute files that libtoolize creates. Finally,
  libtool itself does not provide a bootstrapping script and
  does not explicitly recommend bootstrapping in documentation.

* "Fix libtool installation". We failed to find a good way to
  do that on MacOS (without building and installing newer
  libtool from sources).

* Place m4 files where libtool v2.4 expects to find them.
  That change fixes MacOS builds that use automake v1.17, but
  breaks Gentoo builds because Gentoo libtool installs a buggy
  libltdl/Makefile.in that must be regenerated by automake
  before it can work. Fixing m4 files location prevents that
  regeneration.

We picked the first option despite its drawbacks because the
third option did not work on Gentoo, and asking Squid
developers to install libtool from sources (i.e. the second
option) felt like a greater evil.

This old problem was exposed by recently introduced CI MacOS
tests that started to fail when MacOS brew updated automake
package from v1.16 without the corresponding libtoolize
package changes.

Also work around what seems to be a libtool packaging bug
affecting MacOS/brew environments, including GitHub Actions
runners we use for CI:

    libtool  (2.4.7) : glibtool libtool path :
    /opt/homebrew/bin Bootstrapping glibtoolize:   error:
    creating 'libltdl/configure.ac' ... failed glibtoolize:
    error: creating 'libltdl/configure' ... failed glibtoolize
    failed

That workaround will be removed after libtool package is
fixed.

Also removed a single-iteration "for dir" loop with several
stale hacks from bootstrap.sh: With only two directories to
bootstrap and with a directory-specific mkdir command, source
comments, and progress messages, it is best to unroll that
loop.

----
Backport of PR #1877
@eduard-bagdasaryan
Copy link
Contributor

why Anubis is not merging this v6 PR

This happens because we have different number of checks here and for master. Anubis currently requires 23 required checks to pass. This PR latest b4addc2 commit has 16 checks.

@rousskov
Copy link
Contributor

rousskov commented Oct 1, 2024

why Anubis is not merging this v6 PR

This happens because we have different number of checks here and for master. Anubis currently requires 23 required checks to pass. This PR latest b4addc2 commit has 16 checks.

@kinkie, AFAICT, your options for merging this PR include:

  1. Squash-merge manually on GitHub while making sure to paste the latest PR description as the commit message.
  2. Manually fast-forward v6 to staged commit b4addc2 (on the command line).
  3. Enable "missing" Jenkins(?) tests for v6 branch to allow Anubis to merge automatically.
  4. Ask us to enhance Anubis to support different number of required tests for different target branches (we already have some relevant pending changes, so this is not a big ask, but it may take a few days).

Some of the options highlighted above require additional (i.e. not spelled out) steps.

The ball is on your side.

@rousskov rousskov removed M-waiting-staging-checks https://github.com/measurement-factory/anubis#pull-request-labels M-cleared-for-merge https://github.com/measurement-factory/anubis#pull-request-labels labels Oct 1, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Oct 1, 2024

Manually fast-forwarded into v6

@kinkie kinkie added the M-merged https://github.com/measurement-factory/anubis#pull-request-labels label Oct 1, 2024
@kinkie
Copy link
Contributor Author

kinkie commented Oct 1, 2024

manually merged

@kinkie kinkie closed this Oct 1, 2024
@squid-anubis squid-anubis added the M-failed-other https://github.com/measurement-factory/anubis#pull-request-labels label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M-failed-other https://github.com/measurement-factory/anubis#pull-request-labels M-merged https://github.com/measurement-factory/anubis#pull-request-labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants