[Need Help] Updated: catppuccin-papirus-folders to latest (f83671d) #426978
Conversation
nixpkgs-ci
bot
added
10.rebuild-linux: 1-10
nixpkgs-ci
bot
added
9.needs: reviewer
| cp ${papirus-folders-script} ./papirus-folders | ||
| chmod +x ./papirus-folders |
There was a problem hiding this comment.
Can use install -m to combine these into a single step.
However, we can also pass executable = true to fetchurl and directly run the downloaded file without copying it. See https://nixos.org/manual/nixpkgs/stable/#sec-pkgs-fetchers-fetchurl
| # Per instructions in the papirus-folders project. | ||
| # We fetch specific latest tested commit to ensure reproducibility. | ||
| papirus-folders-script = fetchurl { | ||
| url = "https://raw.githubusercontent.com/PapirusDevelopmentTeam/papirus-folders/0f838ee5679229e3a3e97e3b333c222c9e9615b4/papirus-folders"; |
There was a problem hiding this comment.
To make it easier to update, we could define a rev binding in the let in block:
papirus-folders-rev = "0f838ee5679229e3a3e97e3b333c222c9e9615b4";
papirus-folders-script = fetchurl {
url = "https://raw.githubusercontent.com/PapirusDevelopmentTeam/papirus-folders/${papirus-folders-rev}/papirus-folders";|
|
||
| postPatch = '' | ||
| # Copy the upstream papirus-folders script (pinned), per papirus-folders documentation | ||
| cp ${papirus-folders-script} ./papirus-folders | ||
| chmod +x ./papirus-folders | ||
| patchShebangs ./papirus-folders |
There was a problem hiding this comment.
Likewise, this can be done within fetchurl's postFetch script.
Although, I'm guessing a copy or symlink is still needed if this script is used later in the build. A symlink would be preferred, if possible.
| # This gets stuck on fixup for some reason. Please help. | ||
| dontFixup = true; |
There was a problem hiding this comment.
I can try and investigate. What errors were you seeing?
There was a problem hiding this comment.
A lockup, described in:
It would run a find command during the fixup phase, which would not find anything; and thus the script would error out, but without showing an error in the end.
Unfortunately, verbose mode, nor --show-trace gave me any useful info here.
There was a problem hiding this comment.
As per the general commit conventions and pkgs commit conventions, can you reword the commit to something like:
catppuccin-papirus-folders: 2023-08-02 -> 2024-08-06
From https://github.com/catppuccin/papirus-folders/commit/fcf96737fffc343a1bf129732c37d19f2d77fa5c
To https://github.com/catppuccin/papirus-folders/commit/f83671d17ea67e335b34f8028a7e6d78bca735d7
The most important change is the removal of the `papirus-folders` script from upstream.
Upstream now says “fetch it from papirus-folders.”
If there is a changelog, ideally we'd also link to it.
There was a problem hiding this comment.
Does this imply squashing any amendments?
As the docs say Create one commit for each logical unit.
There was a problem hiding this comment.
If those amendments are fixes for one of the other commits in the PR, then yes you'd squash them.
If it is a logically distinct change, then it's better not to squash. e.g. one commit has a fix for a pre-existing issue in the package and another commit updates to a new version.
|
|
||
| postPatch = '' | ||
| # Copy the upstream papirus-folders script (pinned), per papirus-folders documentation |
There was a problem hiding this comment.
Personally, I would keep all explanation with the fetchurl bit; copying into the build directory is fairly self-explanatory.
Co-authored-by: Matt Sturgeon <[email protected]>
|
Will patch up after I get the rest of the machine set up (on NixOS). |
Updates
catppuccin-papirus-foldersfrom fcf9673 to f83671d (latest).The most important change is the removal of the
papirus-foldersscript from upstream.Upstream now says
fetch it from papirus-folders, so I'm doing an URL fetch on a specific commit (latest at time of writing) to ensure reproducability.Need Help
In this PR I had to set
dontFixup = true, which is likely undesirable.Help is requested here. To the best of my awareness per nixpkgs docs, there are no binaries, man pages, etc. in build output, yet the fixup step locks up. Removing fixup doesn't seem like good practice. So help is requested.
More details here:
I'm not sure how to debug this, as I've only got 3 days experience with Nix(OS) (I haven't even set up my machine fully yet!!), so help would be appreciated.
Aside from that, works as expected, if fixup is skipped.
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.