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.

Sign up for GitHub

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

Jump to bottom

php: Refactor so we can upgrade PHP per platform #47162

Merged
merged 2 commits into from Sep 22, 2018
Merged

php: Refactor so we can upgrade PHP per platform #47162

merged 2 commits into from Sep 22, 2018

Conversation

etu
Copy link
Contributor

@etu etu commented Sep 22, 2018

Motivation for this change

This way we don't need to disable flags etc by platform and can still
backport new versions to stable for linux even if there's a bug or
something in the darwin build.

We would need to trigger a build on darwin as well.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

cc @xeji

@etu
php: Refactor so we can upgrade php per platform
6a28878 
This way we don't need to disable flags etc by platform and can still
backport new versions to stable for linux even if there's a bug or
something in the darwin build.
@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

@GrahamcOfBorg eval

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: php

Partial log (click to expand)

these paths will be fetched (19.00 MiB download, 87.49 MiB unpacked):
  /nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10
copying path '/nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10' from 'https://cache.nixos.org'...
/nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: php

Partial log (click to expand)

  /nix/store/bkvkx0krbbmaay9q7j9pwmf90axgn26y-libargon2-20171227
  /nix/store/jbz0lalq27zxl1xm5pmb0249aw15n7kz-libsodium-1.0.16
  /nix/store/n99jf4qc1xn66zg8pqhxc9wghsyfgp0d-libossp-uuid-1.6.2
  /nix/store/sdq0msyf72h8mbc1x33jracas5331nrd-php-7.2.10
copying path '/nix/store/bkvkx0krbbmaay9q7j9pwmf90axgn26y-libargon2-20171227' from 'https://cache.nixos.org'...
copying path '/nix/store/jbz0lalq27zxl1xm5pmb0249aw15n7kz-libsodium-1.0.16' from 'https://cache.nixos.org'...
copying path '/nix/store/n99jf4qc1xn66zg8pqhxc9wghsyfgp0d-libossp-uuid-1.6.2' from 'https://cache.nixos.org'...
copying path '/nix/store/06ana7c4spn3r1hcc2k08kqkg61xhxfh-postgresql-9.6.10-lib' from 'https://cache.nixos.org'...
copying path '/nix/store/sdq0msyf72h8mbc1x33jracas5331nrd-php-7.2.10' from 'https://cache.nixos.org'...
/nix/store/sdq0msyf72h8mbc1x33jracas5331nrd-php-7.2.10

@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

@GrahamcOfBorg build php71 php72

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: php71, php72

Partial log (click to expand)

these paths will be fetched (18.20 MiB download, 75.91 MiB unpacked):
  /nix/store/bk6j93w39q2kn9zvzw98qc4wnn3zvqq0-php-7.1.22
copying path '/nix/store/bk6j93w39q2kn9zvzw98qc4wnn3zvqq0-php-7.1.22' from 'https://cache.nixos.org'...
/nix/store/bk6j93w39q2kn9zvzw98qc4wnn3zvqq0-php-7.1.22
/nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: php71, php72

Partial log (click to expand)

these paths will be fetched (16.52 MiB download, 72.76 MiB unpacked):
  /nix/store/3dgg1sw9vfpaw26gwlzg4bh8s2ldsbpi-libmcrypt-2.5.8
  /nix/store/pjgmvl45cifa1isbc5q34jl3phgibihn-php-7.1.22
copying path '/nix/store/3dgg1sw9vfpaw26gwlzg4bh8s2ldsbpi-libmcrypt-2.5.8' from 'https://cache.nixos.org'...
copying path '/nix/store/pjgmvl45cifa1isbc5q34jl3phgibihn-php-7.1.22' from 'https://cache.nixos.org'...
/nix/store/pjgmvl45cifa1isbc5q34jl3phgibihn-php-7.1.22
/nix/store/sdq0msyf72h8mbc1x33jracas5331nrd-php-7.2.10

@GrahamcOfBorg
Copy link

Failure on x86_64-darwin (full log)

Attempted: php71, php72

Partial log (click to expand)

/private/tmp/nix-build-php-7.1.21.drv-0/php-7.1.21/main/php_config.h:2548:24: note: expanded from macro 'zend_finite'
#define zend_finite(a) finite(a)
                       ^
/nix/store/qjmqrzk8nhn5maa093fhl3finczz72lw-libc++-5.0.2/include/c++/v1/math.h:439:1: note: 'isfinite' declared here
isfinite(_A1 __lcpp_x) _NOEXCEPT
^
2 errors generated.
make: *** [Makefile:1171: ext/intl/common/common_enum.lo] Error 1
builder for '/nix/store/w0yfdlaydqk3xka0qzg3145rahp6qlyh-php-7.1.21.drv' failed with exit code 2
error: build of '/nix/store/w0yfdlaydqk3xka0qzg3145rahp6qlyh-php-7.1.21.drv' failed

@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

Fails on Darwin...

This is ok as an interim solution but I think it should be reverted asap. I'm not entirely happy with the idea of having platform-dependent versions of packages. Having this split already in the code may tempt maintainers to just ignore build errors on platforms they don't care about, which is probably not what we want.

@etu
Copy link
Contributor Author

etu commented Sep 22, 2018

@xeji Yeah, I agree. But I also don't like the thought of not being upgrade the users on stable linuxes.

I pushed a fixup that we can try with on darwin on php71.

I also sent a PR to ofborg to be able to trigger darwin builds myself...

@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

@GrahamcOfBorg build php71 php72

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: php71, php72

Partial log (click to expand)

  /nix/store/0ch1rvqbarp29rrw6vx5f81dn9vc1pgy-libmcrypt-2.5.8
  /nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10
  /nix/store/bk6j93w39q2kn9zvzw98qc4wnn3zvqq0-php-7.1.22
  /nix/store/ws84zc1i9qi3v9dmi596pjiss63a7fcj-libargon2-20171227
copying path '/nix/store/ws84zc1i9qi3v9dmi596pjiss63a7fcj-libargon2-20171227' from 'https://cache.nixos.org'...
copying path '/nix/store/0ch1rvqbarp29rrw6vx5f81dn9vc1pgy-libmcrypt-2.5.8' from 'https://cache.nixos.org'...
copying path '/nix/store/bk6j93w39q2kn9zvzw98qc4wnn3zvqq0-php-7.1.22' from 'https://cache.nixos.org'...
copying path '/nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10' from 'https://cache.nixos.org'...
/nix/store/bk6j93w39q2kn9zvzw98qc4wnn3zvqq0-php-7.1.22
/nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: php71, php72

Partial log (click to expand)

/nix/store/pjgmvl45cifa1isbc5q34jl3phgibihn-php-7.1.22
/nix/store/sdq0msyf72h8mbc1x33jracas5331nrd-php-7.2.10

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: php

Partial log (click to expand)

/nix/store/sdq0msyf72h8mbc1x33jracas5331nrd-php-7.2.10

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: php

Partial log (click to expand)

/nix/store/0ym4zzr74b3pygkkyvpd3yzm089kzvz0-php-7.2.10

@GrahamcOfBorg
Copy link

Success on x86_64-darwin (full log)

Attempted: php71, php72

Partial log (click to expand)

/nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20/bin/php-config: interpreter directive changed from " /bin/sh" to "/nix/store/cblfnvb5rmhd2z231mqasn0brzh1hhv4-bash-4.4-p23/bin/sh"
/nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20/bin/phpize: interpreter directive changed from "/bin/sh" to "/nix/store/cblfnvb5rmhd2z231mqasn0brzh1hhv4-bash-4.4-p23/bin/sh"
/nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20/lib/build/config.guess: interpreter directive changed from " /bin/sh" to "/nix/store/cblfnvb5rmhd2z231mqasn0brzh1hhv4-bash-4.4-p23/bin/sh"
/nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20/lib/build/config.sub: interpreter directive changed from " /bin/sh" to "/nix/store/cblfnvb5rmhd2z231mqasn0brzh1hhv4-bash-4.4-p23/bin/sh"
/nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20/lib/build/shtool: interpreter directive changed from "/bin/sh" to "/nix/store/cblfnvb5rmhd2z231mqasn0brzh1hhv4-bash-4.4-p23/bin/sh"
moving /nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20/sbin/* to /nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20/bin
strip is /nix/store/df6k4mgdjxciy0f637lryp7c9ln7n1m3-cctools-binutils-darwin/bin/strip
patching script interpreter paths in /nix/store/byxiw5r4w7ii1hd6s1s960z16jpiwj9v-php-7.1.20-dev
/nix/store/wnrjhhzrywkfsvjdlxn7dyvvijh21apq-php-7.1.20
/nix/store/xhyhlplvvr40iss1xqlhx3ly1i2ds7ax-php-7.2.8

@xeji xeji merged commit 0b82fbc into NixOS:master Sep 22, 2018
@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

@etu I squashed the two commits into one, looks nicer.

xeji pushed a commit that referenced this pull request Sep 22, 2018
@etu @xeji
php: Refactor so we can upgrade PHP per platform (#47162)
11aa178 
This way we don't need to disable flags etc by platform and can still
backport new versions to stable for linux even if there's a bug or
something in the darwin build.

(cherry picked from commit 0b82fbc)
@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

I backported this and the preceding updates to 18.09 in 2625469..11aa178 because currently php71 fails to build on darwin for 18.09.

@etu etu deleted the upgrade-php-depending-on-platform branch September 22, 2018 18:31
@etu
Copy link
Contributor Author

etu commented Sep 22, 2018

@etu I squashed the two commits into one, looks nicer.

Yeah, if you wouldn't do that I would before merging. Just wanted to keep track of the changes in between builds. Hence the fixup! :)

I backported this and the preceding updates to 18.09 in 2625469..11aa178 because currently php71 fails to build on darwin for 18.09.

Nice, thanks. Feels good to have working builds. And I will try to upgrade the darwin ones whenever new versions gets released 👍

@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

Squashing during merge is no work at all thanks to Github's nice "squash and merge" button 😄

@etu
Copy link
Contributor Author

etu commented Sep 22, 2018

Hehe, the fixup! and squash! is a special keyword for git rebase --autosquash though :)

@xeji
Copy link
Contributor

xeji commented Sep 22, 2018

cool, didn't know that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@etu @xeji @GrahamcOfBorg