bruno: package from source #261139
bruno: package from source #261139
Conversation
lucasew
force-pushed
the
bruno-src
branch
4 times, most recently
from
91387d8
to
dc298b9
Compare
|
Result of |
1 similar comment
|
Result of |
|
Result of |
|
Result of 1 package built:
|
lucasew
force-pushed
the
bruno-src
branch
2 times, most recently
from
d1201b8
to
39ce9b9
Compare
|
Upstream seems to not care about the package-lock.json thing. So I guess we have to regenerate a patch at each bump. |
|
Result of 1 package built:
|
|
Result of 1 package built:
|
|
Is this PR ready to review? Because in the description it says it is waiting for some issue to be closed first. |
|
Result of 3 packages built:
|
ofborg
bot
added
10.rebuild-darwin: 1-10
and removed
10.rebuild-darwin: 0
10.rebuild-linux: 1
labels
|
Result of 3 packages built:
|
pkgs/by-name/br/bruno/package.nix
Outdated
| let | ||
| electron = electron-bin; | ||
| in |
There was a problem hiding this comment.
Whats the reason behind this?
There was a problem hiding this comment.
Only convenience. I was testing with different electron versions at first and settled for the default one.
| postFetch = '' | ||
| ${lib.getExe npm-lockfile-fix} $out/package-lock.json | ||
| ''; |
There was a problem hiding this comment.
I think we should rather do this in postPatch to make the download easier to reproduce.
Also that this does network requests is not that great. We should probably just apply a patch to get the hashes.
There was a problem hiding this comment.
Build complains about inconsistencies in package-lock.json.
That way the bump script already fixes the lock and updates the hash. As the lock fix is reproducible it's fine.
fetchFromGitHub doesn't have a patchPhase tho
There was a problem hiding this comment.
As the lock fix is reproducible it's fine.
but we now depend on npmjs.com working to be able to fetch the src, right? If someone has a copy of the archive but the service is no longer available or some download is yanked, then the FOD cannot be produced anymore. A patch file shouldn't have that problem.
There was a problem hiding this comment.
We will still depend on npm to generate the node_modules to build the package. BTW for end users this bundle will be cached.
Signed-off-by: lucasew <lucas59356@gmail.com>
lucasew
force-pushed
the
bruno-src
branch
2 times, most recently
from
a845fa1
to
c2a91cf
Compare
|
electron-bin is not "the default". That would be just `electron`.
31 Dec 2023 04:54:27 Lucas Eduardo ***@***.***>:
…
***@***.**** commented on this pull request.
----------------------------------------
In pkgs/by-name/br/bruno/package.nix[#261139 (comment)]:
> +let
+ electron = electron-bin;
+in
Only convenience. I was testing with different electron versions at first and settled for the default one.
—
Reply to this email directly, view it on GitHub[#261139 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AURNSZNG75MWGHXAYQ4EEY3YMDOXDAVCNFSM6AAAAAA6AVDKYOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOJZGYYDMOJSGQ].
You are receiving this because your review was requested.
[Tracking image][https://github.com/notifications/beacon/AURNSZIBCH3FAD56E633DQ3YMDOXDA5CNFSM6AAAAAA6AVDKYOWGG33NNVSW45C7OR4XAZNRKB2WY3CSMVYXKZLTORJGK5TJMV32UY3PNVWWK3TUL5UWJTTLIPJIY.gif]
|
|
Result of 3 packages built:
|
Signed-off-by: lucasew <lucas59356@gmail.com>
Description of changes
Package bruno from source as now they ship package-lock.json together with the repo.
To fix the package-lock.json issue, that upstream doesn't seem to care, I also integrated the tool mentioned in #261137. Using the patch approach requires manual intervention at each bump and generating the package-lock.json is tricky because the tool that fetches src would actually fetch the patched package-lock.json to fix. It's much simpler to integrate the fix into a derivation and also easier bumps using r-ryantm.
I checked if npm-lockfile-fix creates a reproducible result by running the cold build of
bruno.srctwice and it gave the same hash.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/)