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

Fix CC logic in configure #607

Closed
wants to merge 1 commit into from
Closed

Fix CC logic in configure #607

wants to merge 1 commit into from

Conversation

thesamesam
Copy link

@thesamesam thesamesam commented Mar 28, 2022

In e9a52aa,
the logic was changed to try check harder for GCC, but it dropped
the default setting of cc=${CC}. It was throwing away any pre-set CC value as
a result.

The rest of the script then cascades down a bad path because it's convinced
it's not GCC or a GCC-like compiler.

This led to e.g. misdetection of inability to build shared libs
for say, multilib cases (w/ CC being one thing from the environment being used
for one test (e.g. x86_64-unknown-linux-gnu-gcc -m32 and then 'cc' used for
shared libs (but missing "-m32"!)). Obviously just one example of how
the old logic could break.

This restores the old default of 'CC' if nothing overrides it later
in configure.

Bug: https://bugs.gentoo.org/836308
Signed-off-by: Sam James sam@gentoo.org

@thesamesam
Copy link
Author

@madler Could you take a look at this as well as #229 and #599 please? It'd be a big help for distributions who will all be rushing to update soon.

Thanks for the new release.

In e9a52aa,
the logic was changed to try check harder for GCC, but it dropped
the default setting of cc=${CC}. It was throwing away any pre-set CC value as
a result.

The rest of the script then cascades down a bad path because it's convinced
it's not GCC or a GCC-like compiler.

This led to e.g. misdetection of inability to build shared libs
for say, multilib cases (w/ CC being one thing from the environment being used
for one test (e.g. x86_64-unknown-linux-gnu-gcc -m32 and then 'cc' used for
shared libs (but missing "-m32"!)). Obviously just one example of how
the old logic could break.

This restores the old default of 'CC' if nothing overrides it later
in configure.

Bug: https://bugs.gentoo.org/836308
Signed-off-by: Sam James <sam@gentoo.org>
pld-gitsync pushed a commit to pld-linux/zlib that referenced this pull request Mar 28, 2022
@ncopa
Copy link

ncopa commented Mar 28, 2022

I can confirm that this PR solve #608

algitbot pushed a commit to alpinelinux/aports that referenced this pull request Mar 28, 2022
@aeiouaeiouaeiouaeiouaeiouaeiou

This also fixes the compilation of dynamic library in macOS.

algitbot pushed a commit to alpinelinux/aports that referenced this pull request Mar 28, 2022
algitbot pushed a commit to alpinelinux/aports that referenced this pull request Mar 28, 2022
algitbot pushed a commit to alpinelinux/aports that referenced this pull request Mar 28, 2022
algitbot pushed a commit to alpinelinux/aports that referenced this pull request Mar 28, 2022
@madler
Copy link
Owner

madler commented Mar 29, 2022

See 05796d3 .

@madler madler closed this Mar 29, 2022
@thesamesam thesamesam deleted the configure-cc branch March 29, 2022 01:38
@thesamesam
Copy link
Author

Thanks.

@Neustradamus
Copy link

@madler: It is not possible to merge PRs than close?
I think that it is better to have contributor commits...

@madler
Copy link
Owner

madler commented Mar 29, 2022

Not sure how.

@thesamesam
Copy link
Author

thesamesam commented Mar 29, 2022

Not sure how.

I use pram usually (e.g. pram https://github.com/madler/zlib/pull/607), but alternatively:

  1. wget https://github.com/madler/zlib/pull/607.patch -O /tmp/patch-to-review.patch
  2. Review /tmp/patch-to-review.patch
  3. git am /tmp/patch-to-review.patch to apply.

(github has its own merge buttons but I don't usually use those as the repo isn't always hosted on github directly, just a mirror.)

@ncopa
Copy link

ncopa commented Mar 29, 2022

Not sure how.

github normally have a green "merge" button, if you don't care about the merge commits.

Otherwise a shortcut of @thesamesam's suggestion:
curl -L https://github.com/madler/zlib/pull/607.patch | git am -3

@leahneukirchen
Copy link

A convenient way also provided by https://github.com/leahneukirchen/git-merge-pr :)

Mindavi added a commit to Mindavi/nixpkgs that referenced this pull request Apr 5, 2022
Mindavi added a commit to Mindavi/nixpkgs that referenced this pull request Apr 5, 2022
github-actions bot pushed a commit to NixOS/nixpkgs that referenced this pull request Apr 7, 2022
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
thefloweringash pushed a commit to thefloweringash/nixpkgs that referenced this pull request Apr 8, 2022
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
7c6f434c pushed a commit to NixOS/nixpkgs that referenced this pull request Dec 2, 2022
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
awake-bot pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jul 11, 2023
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jul 12, 2023
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jul 13, 2023
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
awake-bot pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jul 20, 2023
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this pull request Jul 21, 2023
Apply patch that already has been applied upstream:
- madler/zlib#607
- madler/zlib@05796d3

(cherry picked from commit f091c0e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants