Navigation Menu

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

Jump to bottom

[Second attempt] php74: init at 7.4.0 #73883

Merged
merged 14 commits into from Dec 4, 2019
Merged

[Second attempt] php74: init at 7.4.0 #73883

merged 14 commits into from Dec 4, 2019

Conversation

etu
Copy link
Contributor

@etu etu commented Nov 21, 2019 •
edited

Motivation for this change

Because of a lot of the mess in #73151, I decided to create my own attempt to package php74 up.

So this is my attempt.

I have taken the minimalist approach. I only changed things that needs to be changed. So only upgraded pecls of things that needed upgrades to work. Added asserts to pecls where upgrades doesn't help etc.

I have built every pecl by hand while testing.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @aanderse @talyz @Izorkin

@etu etu changed the title Init php74 [Second attempt] php74: init at 7.4 RC6 Nov 21, 2019
aanderse
aanderse approved these changes Nov 21, 2019
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

I can confirm php74Packages.oci8 complies. LGTM 👍

Thanks @etu! 🎉

@etu
Copy link
Contributor Author

etu commented Nov 22, 2019

Seems like the following package fails on darwin:
@GrahamcOfBorg build php72Packages.pdo_sqlsrv php73Packages.pdo_sqlsrv php74Packages.pdo_sqlsrv

I just want to see how it behaved on other versions before as well.

@etu
Copy link
Contributor Author

etu commented Nov 22, 2019

Ok, phpPackages.pdo_sqlsrv seems to be broken since before. So this isn't a new problem caused by this PR and there's an assert in place for PHP 7.4 builds of this package. So there's no new breakage here so that shouldn't be a blocker for this PR.

@aanderse
Copy link
Member

@jtojnar look good to you?

@ofborg ofborg bot requested a review from jtojnar November 25, 2019 16:34
talyz
talyz approved these changes Nov 25, 2019
View reviewed changes
Copy link
Contributor

@talyz talyz left a comment

Choose a reason for hiding this comment

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

Looks good with the latest fixes!

@etu
Copy link
Contributor Author

etu commented Nov 25, 2019

@GrahamcOfBorg build php72Packages.pdo_sqlsrv php73Packages.pdo_sqlsrv

These build fine locally.

@etu etu changed the title [Second attempt] php74: init at 7.4 RC6 [Second attempt] php74: init at 7.4.0 Nov 27, 2019
@etu
Copy link
Contributor Author

etu commented Nov 27, 2019

@aanderse @talyz The release announcement haven't been published yet. But they seem to have an uploaded the tarball.

This is not uncommon for PHP 🙂

But we should wait for the announcement and see that they don't change the tarball before the release is announced. But we can test further until then.

@etu
Copy link
Contributor Author

etu commented Nov 28, 2019

PHP 7.4.0 is now released.

The sha256 of the tarball is the same as yesterday.

talyz
talyz approved these changes Nov 30, 2019
View reviewed changes
Copy link
Contributor

@talyz talyz left a comment

Choose a reason for hiding this comment

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

The latest version builds fine and the binaries seem to work 👍

@aanderse
Copy link
Member

Anyone object to a merge at this point?

@Izorkin
Copy link
Contributor

Izorkin commented Nov 30, 2019

Not fixed this warning?

configure: WARNING: unrecognized options: --with-pcre-regex, --with-libxml-dir, --with-gd, --with-freetype-dir, --with-png-dir, --with-jpeg-dir, --enable-zip, --with-libzip
checking for grep that handles long lines and -e... /nix/store/20b535jb98hy7k4z8vkrlkjma212a3l5-gnugrep-3.3/bin/grep

@etu
Copy link
Contributor Author

etu commented Dec 4, 2019 •
edited

@Izorkin I've resolved that in a4cf20c after your comment.

But I made sure to not affect older versions. So we're not triggering rebuilds of either php72 or php73.

@aanderse
Copy link
Member

aanderse commented Dec 4, 2019

Thanks for being awesome @etu!

@aanderse aanderse merged commit 4b995dd into NixOS:master Dec 4, 2019
dtzWill pushed a commit to dtzWill/nixpkgs that referenced this pull request Dec 5, 2019
@aanderse @dtzWill
Merge pull request NixOS#73883 from etu/init-php74
925d5fe 
php74: init at 7.4.0
(cherry picked from commit 4b995dd)
@etu etu deleted the init-php74 branch December 5, 2019 06:15
@etu etu mentioned this pull request Dec 5, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Izorkin Izorkin Izorkin left review comments

@talyz talyz talyz approved these changes

@aanderse aanderse aanderse approved these changes

@CMCDragonkai CMCDragonkai Awaiting requested review from CMCDragonkai

@globin globin Awaiting requested review from globin globin is a code owner

@offlinehacker offlinehacker Awaiting requested review from offlinehacker

@javaguirre javaguirre Awaiting requested review from javaguirre

@jtojnar jtojnar Awaiting requested review from jtojnar jtojnar is a code owner

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@etu @aanderse @Izorkin @talyz @jtojnar