symfony-cli: add missing compilation flags #206149
Conversation
|
Hi, thanks for this! Question, do you think it would be possible to use go realaser and have reproducible builds ? |
You are welcome.
I would not bet it. I don’t think it’s a kind of software with reproducibility in mind. But the truth is I can’t tell because I never used it. Anyway, I would prefer relying on nixpkgs’ |
They do it on their side in the Github Action file: https://github.com/symfony-cli/symfony-cli/blob/main/.github/workflows/releaser.yml |
|
Just because they do that doesn't make them build in a reproducible way. Note they also add a build date… :p |
Fair enough ! I validated this PR, all good for me then. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
| ldflags = [ | ||
| "-s" | ||
| "-w" | ||
| "-X main.version=${version}" |
There was a problem hiding this comment.
Are not these two different flags?
| "-X main.version=${version}" | |
| "-X" "main.version=${version}" |
There was a problem hiding this comment.
I don’t really know if it should be considered as two different flags. If we wanted to add something like main.channel it would result in :
ldflags = [
"-s"
"-w"
"-X"
"main.version=${version}"
"-X"
"main.channel=${channel}"
]
instead of :
ldflags = [
"-s"
"-w"
"-X main.version=${version}"
"-X main.channel=${channel}"
]
I really don’t know what is the best for NixOS
There was a problem hiding this comment.
The question is whether it works if you run it in shell.
The stdenv.mkDerivation does not really support lists by default so currently, it will end up being run like go install "-ldflags=-s -w -X main.version=x.y.z -X main.channel=foo":
nixpkgs/pkgs/build-support/go/module.nix
Line 211 in fbcb61b
But in the future, when we switch to structured attributes, it would be good if arguments would be properly split.
| postInstall = '' | ||
| mv $out/bin/symfony-cli $out/bin/symfony | ||
| ''; | ||
|
|
||
| # Tests requires network access | ||
| doCheck = false; | ||
| checkPhase = '' | ||
| $GOPATH/bin/symfony-cli |
There was a problem hiding this comment.
When overriding checkPhase, you should also provide runHook preCheck and runHook postCheck. See https://github.com/jtojnar/nixpkgs-hammering/blob/master/explanations/missing-phase-hooks.md for more info.
Though, in this case, it might be better to use passthru.tests instead of replacing checkPhase so that it can be verified it works outside of the build environment (in a cleaner environment, more representative of actual users’ systems). And also so that we can re-instate doCheck when we manage to fix it.
nixpkgs/pkgs/tools/misc/osm2pgsql/default.nix
Lines 49 to 51 in 8bc6f34
There was a problem hiding this comment.
@jtojnar I can provide an updated PR but I would need more info on this.
Description of changes
I noticed that version displayed by the symfony-cli command line is always
dev:whereas official binaries from symfony-cli repository says
5.4.19, for instance.I tried to figure out why and it’s actually due to some differences on compilation flags between official build and the current NixOS nix expression.
I tried to get closer of the official way, but without adding a build date to keep reproducibility.
I also added a simple check to be sure that the command can run.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes