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

Skip building XS when PUREPERL_ONLY=1 set #2

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zmughal
Copy link

@zmughal zmughal commented May 26, 2021

This change skips the static/dynamic sections of the Makefile when using a
pure-Perl install so that no XS linking is done at all.

Compare the build logs below:

$ for br in master pureperl-skip-linking; do \
echo "On branch $br"; \
git checkout -q $br ; \
git clean -f; \
( perl Makefile.PL PUREPERL_ONLY=1 && make && make clean)  | grep -Fe $( perl -MConfig -e 'print $Config{cc}' ); \
echo; \
done
On branch master
Checking for cc... cc
cc -c   -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -O2   -DVERSION=\"1.102\" -DXS_VERSION=\"1.102\" -fPIC "-I$HOME/perl5/perlbrew/perls/perl-5.26.1/lib/5.26.1/x86_64-linux/CORE"   Util.c
cc  -shared -O2 -L/usr/local/lib -fstack-protector-strong  Util.o  -o blib/arch/auto/Params/Util/Util.so  \

On branch pureperl-skip-linking
Checking for cc... cc

@zmughal
Copy link
Author

zmughal commented May 26, 2021

I see there is not a CI configuration running any more (previously Travis CI). Would it help to use GitHub Actions to test this out?

@rehsack
Copy link
Member

rehsack commented May 26, 2021

I would love a PR which test whether this provides a reasonable and working change :)
Further, I'd prefer commit messages which explain the business case behind a commit, not the technical change. Most people reading commits can derive technical stuff on it's own and the remaining need the business reasons.

When `PUREPERL_ONLY=1` set, this prevents the Makefile from having rules
that compile the XS code. This is useful to have when building
dependencies for deploying to a pure Perl install such as when using
`App::FatPacker`. This removes the need to skip over files that end in
`perl -V:dlext` (e.g., `.so`, `.dll`, `.dylib`) when copying to a
deployment.

This should allow for using the `cpanm --pureperl` option so that a pure
Perl installation can be requested for all installed distributions.

Signed-off-by: Zakariyya Mughal <[email protected]>
@zmughal zmughal force-pushed the pureperl-skip-linking branch 14 times, most recently from 31e9276 to 4c9b0c9 Compare May 26, 2021 19:15
@zmughal
Copy link
Author

zmughal commented May 26, 2021

@rehsack, OK, sounds good!

You can review updated commits to see if this fits the requirements. A workflow that tests what I'm getting at is here.

@zmughal
Copy link
Author

zmughal commented Jun 25, 2021

Hi, if you have time, could you review this. It should be working on multiple platforms per the new CI workflow.

@zmughal
Copy link
Author

zmughal commented Dec 5, 2021

@rehsack, just reaching out again about this PR.

@zmughal
Copy link
Author

zmughal commented Jan 27, 2022

@rehsack, hello, do you have a moment to review this PR?

This is to test that the tests pass as expected under various versions
of Perl both using XS and pure Perl installs.

Signed-off-by: Zakariyya Mughal <[email protected]>
@zmughal zmughal force-pushed the pureperl-skip-linking branch from 4c9b0c9 to 3afa712 Compare August 9, 2022 23:38
@zmughal
Copy link
Author

zmughal commented Aug 9, 2022

Hello @rehsack, I just ran the CI I made here again and it still passes. I also added a few more Perl versions to test against.

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.

2 participants