mas: 1.8.6 -> 1.9.0 #379858
mas: 1.8.6 -> 1.9.0 #379858
Conversation
github-actions
bot
added
10.rebuild-darwin: 1
10.rebuild-darwin: 1-10
10.rebuild-linux: 0
|
I left the reformatting in a separate commit so that the first commit showing the actual changes can be reviewed more easily. |
pkgs/by-name/ma/mas/package.nix
Outdated
| "aarch64-darwin" | ||
| ]; | ||
| maintainers = with maintainers; [ steinybot zachcoyle ]; | ||
| platforms = [ "x86_64-darwin" "aarch64-darwin" ]; |
There was a problem hiding this comment.
I considered this, but it was changed from lib.platforms.darwin in e55eb3d#diff-e9cddb1ec8d2c67121f7529df3f0326832437dea4108f3640057e02053ad5227L36. I assumed it was because the other -darwin systems aren’t supported.
| testers, | ||
| mas, | ||
| }: | ||
| { lib, stdenvNoCC, fetchurl, libarchive, p7zip, testers, mas, }: | ||
|
|
||
| stdenvNoCC.mkDerivation rec { |
There was a problem hiding this comment.
Please don’t hassle people about this kind of thing on unrelated PRs that don’t introduce new instances of patterns some people dislike.
There was a problem hiding this comment.
@ethancedwards8 Since you thumbsed my comment down I would like to encourage you to look at the contributor guidelines for reviewing PRs and the relevant discussions in #370949 and #264651. Nitpicking unrelated lines in files touched by PRs that don’t introduce new instances of problematic patterns, are orthogonal to the subject of the nitpick, and don’t already do any other substantive refactoring is generally inappropriate, as it discourages contributors and promotes a culture of bikeshedding and nitpicking over substantive review. This applies especially in the case of something like finalAttrs which is not always unambiguously superior to the alternatives. Automated linting and treewides are better avenues for this kind of thing.
There was a problem hiding this comment.
Please drop the formatting commit; the file was already formatted according to nixfmt-rfc-style rules, whereas it looks like you have formatted it with nixfmt-classic. Yes, confusing, I know; the error tells you how to get the right formatter.
Other than that and the comment I’ve left, this looks good to me. No need to handle trivial stuff like finalAttrs/meta.platforms in an unrelated PR.
pkgs/by-name/ma/mas/package.nix
Outdated
| installPhase = '' | ||
| install -D './${version}/bin/mas' "$out/bin/mas" | ||
| installShellCompletion --cmd mas --bash './${version}/etc/bash_completion.d/mas' | ||
| mkdir -p $out/bin | ||
| cp mas $out/bin | ||
| ''; |
There was a problem hiding this comment.
Preferably add runHook preInstall to the start and runHook postInstall to the end, but not blocking as it was already like this.
However, this is losing the shell completion. Hopefully it’s present in the .pkg and we can install it (and the Fish one too?); see Homebrew’s formula. Otherwise, it might be best to stick with the Homebrew bottle for now.
We should be able to build this from source and I might take a look at doing so another time.
There was a problem hiding this comment.
Unfortunately, no. As I mention in the description, the pkg doesn’t contain any completion files. It looks like the project has never included the completions in the .pkg files, and the tarballs aren’t produced for newer versions. So it looks like we’d wait until this is built from source.
Notably, this switches back to the .pkg from the tarball, since mas-cli/mas#452 was fixed about three years ago (just after this package was last bumped) and tarballs are no longer published. Also, the Bash completion file isn’t included in the pkg (but it’s still in the repo). Here are the intervening release notes: - https://github.com/mas-cli/mas/releases/tag/v1.8.7 - https://github.com/mas-cli/mas/releases/tag/v1.8.8 - https://github.com/mas-cli/mas/releases/tag/v1.9.0 And the list of (almost 600) commits since the version currently in Nixpkgs: mas-cli/mas@v1.8.6...v1.9.0
pkgs/by-name/ma/mas/package.nix
Outdated
| "aarch64-darwin" | ||
| ]; | ||
| maintainers = with maintainers; [ steinybot zachcoyle ]; | ||
| platforms = [ "x86_64-darwin" "aarch64-darwin" ]; |
There was a problem hiding this comment.
I considered this, but it was changed from lib.platforms.darwin in e55eb3d#diff-e9cddb1ec8d2c67121f7529df3f0326832437dea4108f3640057e02053ad5227L36. I assumed it was because the other -darwin systems aren’t supported.
pkgs/by-name/ma/mas/package.nix
Outdated
| installPhase = '' | ||
| install -D './${version}/bin/mas' "$out/bin/mas" | ||
| installShellCompletion --cmd mas --bash './${version}/etc/bash_completion.d/mas' | ||
| mkdir -p $out/bin | ||
| cp mas $out/bin | ||
| ''; |
There was a problem hiding this comment.
Unfortunately, no. As I mention in the description, the pkg doesn’t contain any completion files. It looks like the project has never included the completions in the .pkg files, and the tarballs aren’t produced for newer versions. So it looks like we’d wait until this is built from source.
wegank
added
the
12.approvals: 1
Notably, this switches back to the .pkg from the tarball, since mas-cli/mas#452 was fixed about three years ago (just after this package was last bumped) and tarballs are no longer published. Also, the Bash completion file isn’t included in the pkg (but it’s still in the repo).
Here are the intervening release notes:
And the list of (almost 600) commits since the version currently in Nixpkgs: mas-cli/mas@v1.8.6...v1.9.0
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.