-
Notifications
You must be signed in to change notification settings - Fork 59
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
Creation of PHAR using Nix, in a reproducible environment #754
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need some more clarity on the degree of manual intervention on flake.nix
: can't really manually change its contents at every release
Using PHP-Scoper create issue with the `assert` function
flake.nix
Outdated
# This only changes when `composer.lock` is updated | ||
vendorHash = "sha256-LsrGmver7RyiI0/l2j6dZaqhFQf2OFyUOZb8xzFFEIA="; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so we're back at having the hash here: how would one approach a patch by Renovate where composer.lock
is being touched? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... I can't basically get rid of it, I tried... and there's a reason to have it there.
Since this PR is quite messy, how about lively discuss this during an informal meeting at your best convenience?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this PR is quite messy, how about lively discuss this during an informal meeting at your best convenience?
Can probably do next week about this.
Meanwhile, I have an example of a repo that has a Cargo.lock
and no hash: https://github.com/Ocramius/oci-srm-server-mock-rust/blob/383e9bd1e99fca42578bd885ae01ff7c74b163ed/flake.nix
Perhaps php.buildComposerProject
to be improved? Or are we missing sha256 for downloaded vendor projects as part of Composer itself? I see that Cargo.lock
has checksums, while we don't
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: could be worked around by assuming --prefer-source
to download via git
, where we at least have a SHA1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do that next week then, ping me on slack, matrix or even telegram when you're ready.
Regarding the builder, there might be room for improvements for sure, and what you said is actually something I tried to fix in NixOS/nixpkgs#225401
I wish I could have a mapping between the composer.lock
file and the vendorHash
... But I have the feeling that it would require to update Composer first. That's an interesting discussion for sure.
You can find the sources of the builder here: https://github.com/NixOS/nixpkgs/tree/master/pkgs/build-support/php
Feel free to suggest improvements, having you on board on this is definitely a big plus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I have the feeling that it would require to update Composer first.
If we only do source-based installs, we can rely on git
for some basic shasum, since Composer's is currently useless.
For now, I'd be happy if in this patch the vendorHash
could be generated inside the github action: that would be a sufficient approach to move forward, IMO
This was taken from PHPStan to reset the PHAR files timestamps.
Box 4.6.0 is now on `nixos-unstable` branch, there's no need to have an extra input of `master` branch.
remove the derivation using `buildComposerProject` and create a build-phar-script with Nix instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚢 thanks @drupol!
I know this isn't exactly what you wanted, but I'm also glad that we found a middle-ground here, and that we introduced nix and nix-flakes in the entire dev stack.
Happy that the discussions/requirements led to more understanding of what package maintainers need from this end too :-)
I'll now merge and try out the release script, thanks for the help!
Aw, still need to fix PHP 8.1 :D Should be trivial, though: I'm on it. |
The CI failure is due to an ancient build of the require-checker Unsure why we |
Well, this is merged, but no phar got tagged :D https://github.com/Roave/BackwardCompatibilityCheck/actions/workflows/release-phar.yml I suspect the release being triggered by a bot prevents this task from being triggered: BackwardCompatibilityCheck/.github/workflows/release-phar.yml Lines 5 to 8 in 4f6c882
|
The publishing payload, meanwhile, taken from https://api.github.com/repos/roave/BackwardCompatibilityCheck/events : {
"id": "34310185998",
"type": "ReleaseEvent",
"actor": {
"id": 41898282,
"login": "github-actions[bot]",
"display_login": "github-actions",
"gravatar_id": "",
"url": "https://api.github.com/users/github-actions[bot]",
"avatar_url": "https://avatars.githubusercontent.com/u/41898282?"
},
"repo": {
"id": 108544542,
"name": "Roave/BackwardCompatibilityCheck",
"url": "https://api.github.com/repos/Roave/BackwardCompatibilityCheck"
},
"payload": {
"action": "published",
"release": {
"url": "https://api.github.com/repos/Roave/BackwardCompatibilityCheck/releases/134622383",
"assets_url": "https://api.github.com/repos/Roave/BackwardCompatibilityCheck/releases/134622383/assets",
"upload_url": "https://uploads.github.com/repos/Roave/BackwardCompatibilityCheck/releases/134622383/assets{?name,label}",
"html_url": "https://github.com/Roave/BackwardCompatibilityCheck/releases/tag/8.5.0",
"id": 134622383,
"author": {
"login": "github-actions[bot]",
"id": 41898282,
"node_id": "MDM6Qm90NDE4OTgyODI=",
"avatar_url": "https://avatars.githubusercontent.com/in/15368?v=4",
"gravatar_id": "",
"url": "https://api.github.com/users/github-actions%5Bbot%5D",
"html_url": "https://github.com/apps/github-actions",
"followers_url": "https://api.github.com/users/github-actions%5Bbot%5D/followers",
"following_url": "https://api.github.com/users/github-actions%5Bbot%5D/following{/other_user}",
"gists_url": "https://api.github.com/users/github-actions%5Bbot%5D/gists{/gist_id}",
"starred_url": "https://api.github.com/users/github-actions%5Bbot%5D/starred{/owner}{/repo}",
"subscriptions_url": "https://api.github.com/users/github-actions%5Bbot%5D/subscriptions",
"organizations_url": "https://api.github.com/users/github-actions%5Bbot%5D/orgs",
"repos_url": "https://api.github.com/users/github-actions%5Bbot%5D/repos",
"events_url": "https://api.github.com/users/github-actions%5Bbot%5D/events{/privacy}",
"received_events_url": "https://api.github.com/users/github-actions%5Bbot%5D/received_events",
"type": "Bot",
"site_admin": false
},
"node_id": "RE_kwDOBnhCHs4IBiyv",
"tag_name": "8.5.0",
"target_commitish": "8.5.x",
"name": "8.5.0",
"draft": false,
"prerelease": false,
"created_at": "2023-12-20T13:08:55Z",
"published_at": "2023-12-20T13:08:57Z",
"assets": [
],
"tarball_url": "https://api.github.com/repos/Roave/BackwardCompatibilityCheck/tarball/8.5.0",
"zipball_url": "https://api.github.com/repos/Roave/BackwardCompatibilityCheck/zipball/8.5.0",
"body": "### Release Notes for [8.5.0](https://github.com/Roave/BackwardCompatibilityCheck/milestone/59)\n\nFeature release (minor)\n\n### 8.5.0\n\n- Total issues resolved: **0**\n- Total pull requests resolved: **2**\n- Total contributors: **2**\n\n#### enhancement\n\n - [755: Fix: Consistently indent with 2 spaces](https://github.com/Roave/BackwardCompatibilityCheck/pull/755) thanks to @localheinz\n - [754: Creation of PHAR using Nix, in a reproducible environment](https://github.com/Roave/BackwardCompatibilityCheck/pull/754) thanks to @drupol\n",
"mentions_count": 2,
"mentions": [
{
"avatar_url": "https://avatars.githubusercontent.com/u/252042?v=4",
"login": "drupol",
"profile_name": "Pol Dellaiera",
"profile_url": "https://github.com/drupol",
"avatar_user_actor": true
},
{
"avatar_url": "https://avatars.githubusercontent.com/u/605483?v=4",
"login": "localheinz",
"profile_name": "Andreas Möller",
"profile_url": "https://github.com/localheinz",
"avatar_user_actor": true
}
],
"short_description_html": "<h3>Release Notes for <a href=\"https://github.com/Roave/BackwardCompatibilityCheck/milestone/59\">8.5.0</a>\n</h3>\n<p>Feature release (minor)</p>\n<h3>8.5.0</h3>\n<ul>\n<li>Total issues resolved: <strong>0</strong>\n</li>\n<li>Total pull requests resolved: <strong>2</strong>\n</li>\n<li>Total contributors: <strong>2</strong>\n</li>\n</ul>\n<h4>enhancement</h4>\n<ul>\n<li>\n<a href=\"https://github.com/Roave/BackwardCompatibilityCheck/pull/755\" data-hovercard-type=\"pull_request\" data-hovercard-url=\"/Roave/BackwardCompatibilityCheck/pull/755/hovercard\">755: Fix: Consistently indent with 2 spaces</a> thank…</li>\n</ul>",
"is_short_description_html_truncated": true
}
},
"public": true,
"created_at": "2023-12-20T13:08:57Z",
"org": {
"id": 3029050,
"login": "Roave",
"gravatar_id": "",
"url": "https://api.github.com/orgs/Roave",
"avatar_url": "https://avatars.githubusercontent.com/u/3029050?"
}
}, |
Let me have a look at the workflow at the end of the day |
Identified that I need
|
Now it did run, but failed :D
https://github.com/Roave/BackwardCompatibilityCheck/actions/runs/7276447892 That's... curious |
Hey there,
I saw the message from @sebastianbergmann on Mastodon (https://main.elk.zone/phpc.social/@sebastian/111528636893710027) and I thought... How about using Nix to do it?
This PR shows how it could be done using 2 different approaches:
build-phar.sh
and nixifying it, providingbox
without needing to install it with Composer and build the PHAR in a non isolated environment.flake.nix
filenix build .#build-phar-script
to build the script to build the PHAR (result inresult/
directory)nix build .#phar
to build the PHAR in isolation (result inresult/
directory)nix run github:drupol/BackwardCompatibilityCheck/creation-of-phar-using-nix
to immediately run the appfrom your workstation without installing it, in total isolation.
nix develop
to create a development shell containing PHP, Composer and Box and thebuild-phar-script
Both methods work well, and are totally reproducible. My favourite approach is to use the second one which is more flexible and versatile, but anyway one or the other, the environment building the PHAR is the same... (pun intended!). Note, the PHAR is not reproducible,
box
doesn't produce reproducible PHAR files by default, unfortunately (read how to make it reproducible here)I'm pretty sure this PR won't be merged, but I just wanted to see if this was doable, I hope this is going to give ideas to some PHP community members :)
It took me 25 minutes to do, mostly copy-pasting things here and there.