Skip to content

macOS related patches for 10.45#668

Merged
NWilson merged 2 commits intoPCRE2Project:masterfrom
carenas:macos
Jan 13, 2025
Merged

macOS related patches for 10.45#668
NWilson merged 2 commits intoPCRE2Project:masterfrom
carenas:macos

Conversation

@carenas
Copy link
Contributor

@carenas carenas commented Jan 12, 2025

A couple of "fixes" that came handy while building the snapshot in macOS (using an ARM CPU)

First one does just a aesthetic cleanup but has no side effects and is therefore likely safe.

Second one provides a mechanism for using gcc instead of clang and therefore also provides some minimal "fixes" to the codebase for building with an old version of it with all the extra warnings enabled.

likely added by mistake, the functionality works through `--enable-debug`
instead.
Instead of hardcoding the compiler as `cc`, let a CC environment
variable dictate which compiler to use.

For example, in macOS/arm64 where the GNU compiler is provided by
brew the following will allow using it instead of the system compiler
(which ALSO answers to `gcc` eventhough is `clang`)

  % CC=gcc-13 maint/ManyConfigTests
Copy link
Collaborator

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@NWilson NWilson left a comment

Choose a reason for hiding this comment

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

It appears Philip added several AM_CONDITIONALs without actually making use of them. They are harmless if not used, but I'm happy for them to be cleaned up.

As far as I'm aware, they don't affect the commandline options accepted by ./configure; instead they define which variables are substituted inside Makefile.in. So if an AM_CONDITIONAL isn't referenced in Makefile.in, then it has no effect.

@NWilson
Copy link
Member

NWilson commented Jan 13, 2025

I apologize for the build failure in "Check autogenerated file freshness". You may ignore that. It seems I broke something by mistake over the weekend. I will fix it.

Comment on lines +118 to 119
$CC --version >/tmp/pcre2ccversion 2>/dev/null
if [ $? -eq 0 ] && grep GCC /tmp/pcre2ccversion >/dev/null; then
Copy link
Member

Choose a reason for hiding this comment

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

Very curiously, on an Ubuntu machine, cc --version does not match "GCC", nor does gcc --version (it says "gcc" lowercase), nor does clang --version.

Hmm. I should check it's all OK in the CI!

Copy link
Member

Choose a reason for hiding this comment

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

I found that the CI is not using all the GCC-specific warnings in the ManyConfigTests job! Oops. As you'd expect, the are some warnings that have crept in. I've made a separate PR for it: #670

@PhilipHazel
Copy link
Collaborator

t appears Philip added several AM_CONDITIONALs without actually making use of them. They are harmless if not used, but I'm happy for them to be used up.

I regret that I cannot remember the details of this, only that the building apparatus has been heavily hacked about over the decades. I am not surprised to learn that there is detritus in there.

@NWilson NWilson merged commit 95181ff into PCRE2Project:master Jan 13, 2025
@carenas carenas deleted the macos branch January 13, 2025 17:35
NWilson pushed a commit that referenced this pull request Feb 4, 2025
* autotools: retire conditional for debug build

likely added by mistake, the functionality works through `--enable-debug`
instead.

* maint: allow selecting compiler for ManyConfigTests

Instead of hardcoding the compiler as `cc`, let a CC environment
variable dictate which compiler to use.

For example, in macOS/arm64 where the GNU compiler is provided by
brew the following will allow using it instead of the system compiler
(which ALSO answers to `gcc` even though is `clang`)

  % CC=gcc-13 maint/ManyConfigTests
@NWilson NWilson mentioned this pull request Feb 4, 2025
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.

4 participants