Skip to content
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

qmidiarp: init at 0.6.5 #82795

Merged
merged 1 commit into from Mar 27, 2020
Merged

qmidiarp: init at 0.6.5 #82795

merged 1 commit into from Mar 27, 2020

Conversation

@sjfloat
Copy link
Contributor

sjfloat commented Mar 17, 2020

Motivation for this change

Introduction of qmidiarp app.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip" New pkg, no dependents.
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Copy link
Contributor

symphorien left a comment

Thanks for your PR. I left some comments to improve it a bit.

pkgs/applications/audio/qmidiarp/default.nix Outdated Show resolved Hide resolved
@sjfloat
Copy link
Contributor Author

sjfloat commented Mar 17, 2020

Do the checks reveal that I need to correct the sha256? It builds for me OK locally, but I'm not sure it aligns with the rev being used. Should it?

@symphorien
Copy link
Contributor

symphorien commented Mar 17, 2020

Try to replace the hash to all zeroes, and then rebuild. You should get the same good hash than in the error message.

@sjfloat
Copy link
Contributor Author

sjfloat commented Mar 17, 2020

Try to replace the hash to all zeroes, and then rebuild. You should get the same good hash than in the error message.

Nice tip. nix-prefetch-git accepts a --rev argument, so I think I got the right one now. Note that I'm not getting an error message with either one building locally. But one of the @ofborg tests gave errors (though it passed everthing); this is what brought it to my attention.

@symphorien
Copy link
Contributor

symphorien commented Mar 17, 2020

Yes, if you have downloaded something (possibly unrelated) with this hash in the past, nix won't even try to download it again, even if you change the url.

@sjfloat
Copy link
Contributor Author

sjfloat commented Mar 17, 2020

BTW, I'm still not able to request reviews -- it's greyed out. I could ping @jonringer or @jtojnar, but I don't want to overburden them. Is there a more general way?

@symphorien
Copy link
Contributor

symphorien commented Mar 17, 2020

Huh I don't really know the inner workings or github...

@jtojnar
Copy link
Contributor

jtojnar commented Mar 17, 2020

Do the checks reveal that I need to correct the sha256? It builds for me OK locally, but I'm not sure it aligns with the rev being used. Should it?

Using nix-build -A foo.src --check might also work.

@sjfloat
Copy link
Contributor Author

sjfloat commented Mar 17, 2020

Do the checks reveal that I need to correct the sha256? It builds for me OK locally, but I'm not sure it aligns with the rev being used. Should it?

Using nix-build -A foo.src --check might also work.

Nice. I need to remember that!

I'll push up with the corrected hash.

@sjfloat sjfloat force-pushed the sjfloat:qmidiarp branch from 32f1ded to 75b4ce9 Mar 17, 2020
@sjfloat
Copy link
Contributor Author

sjfloat commented Mar 17, 2020

I checked the data files produced by the app before and after adjusting the hash. It was indeed pulling from master. It's correct now.

@sjfloat sjfloat requested review from jtojnar and symphorien Mar 17, 2020
Copy link
Contributor

symphorien left a comment

  • Compiles locally
  • Runs fine, even in an empty PATH
@jonringer
Copy link
Contributor

jonringer commented Mar 17, 2020

Nice. I need to remember that!

You can also just put in an invalid sha (I usually just change the last character to a).

This may seem like a hack (it is), but some fetch helpers such as fetchpatch will do some processing on the contents and change the contents before determining the sha, and if you do a nix-prefetch, you'll get cached hits (the sha before fetchpatch does its processing).

Copy link
Contributor

jonringer left a comment

diff LGTM
shows usage

[3 built, 1 copied (1.1 MiB), 0.2 MiB DL]
https://github.com/NixOS/nixpkgs/pull/82795
1 package built:
qmidiarp
@jonringer
Copy link
Contributor

jonringer commented Mar 17, 2020

@GrahamcOfBorg build qmidiarp

@sjfloat
Copy link
Contributor Author

sjfloat commented Mar 20, 2020

@jonringer is there anything holding this up at this point?

@jonringer
Copy link
Contributor

jonringer commented Mar 27, 2020

no, I was just busy, sorry

@jonringer jonringer merged commit 28aa914 into NixOS:master Mar 27, 2020
16 checks passed
16 checks passed
qmidiarp on x86_64-darwin No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
qmidiarp on aarch64-linux Success
Details
qmidiarp on x86_64-linux Success
Details
@sjfloat
Copy link
Contributor Author

sjfloat commented Mar 27, 2020

no, I was just busy, sorry

No apologies! You're doing a great job!

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.