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

rmapi: init at 0.0.11 #85898

Merged
merged 1 commit into from May 25, 2020
Merged

rmapi: init at 0.0.11 #85898

merged 1 commit into from May 25, 2020

Conversation

@NickHu
Copy link
Contributor

NickHu commented Apr 23, 2020

Motivation for this change

Add the rMAPI tool.

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"
  • 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.
@doronbehar
Copy link
Contributor

doronbehar commented May 24, 2020

Please switch to vendorSha256, this should fix also the CI error.

@doronbehar
Copy link
Contributor

doronbehar commented May 24, 2020

Oh I see you commented at ryantm/nixpkgs-update#203, I think it'll work out for you if you'd not only rename the variable modSha256 to vendorSha256, but also zero it out before you rerun nix-build.

@doronbehar
Copy link
Contributor

doronbehar commented May 25, 2020

This works:

diff --git i/pkgs/applications/misc/remarkable/rmapi/default.nix w/pkgs/applications/misc/remarkable/rmapi/default.nix
index 66be455a040..0fe0e03060a 100644
--- i/pkgs/applications/misc/remarkable/rmapi/default.nix
+++ w/pkgs/applications/misc/remarkable/rmapi/default.nix
@@ -11,7 +11,7 @@ buildGoModule rec {
     sha256 = "0zks1pcj2s2pqkmw0hhm41vgdhfgj2r6dmvpsagbmf64578ww349";
   };
 
-  modSha256 = "0w2qiafs5gkgv00yz16bx8yis6gnpxbgqliwrhj5k6z8yy9s7b17";
+  vendorSha256 = "077s13pcql5w2m6wzls1q06r7p501kazbwzxgfh6akwza15kb4is";
 
   meta = with stdenv.lib; {
     description = "A Go app that allows access to the ReMarkable Cloud API programmatically";
@doronbehar
Copy link
Contributor

doronbehar commented May 25, 2020

@Enteee, personally, I like this PR much more then yours - it's just so much cleaner and regular. I think I've sort of understood now what was your motivation behind the IFD thing, it's just that I didn't catch initially that you tried to push Nix support upstream at juruen/rmapi#78 .

Never the less, I'm still a bit puzzled - almost every Go app that we ship doesn't include default.nix, shell.nix and all of those files, and most people seem happy about it. Even packages which do have these files and naturally are maintained both upstream and downstream by Nixpkgs contributors, are not packaged using IFD hooks such as you want to apply in #74657. Examples: https://github.com/NixOS/nixpkgs/search?q=maintainers.mic92&unscoped_q=maintainers.mic92

I think you'll like the yet unstable feature flakes - NixOS/rfcs#49 . When it'll stabilized, it'll be possible to do cool stuff like you tried in #74657 with less workarounds.

@NickHu NickHu force-pushed the NickHu:rmapi branch from 9a6ed30 to d5c52db May 25, 2020
@NickHu
Copy link
Contributor Author

NickHu commented May 25, 2020

@doronbehar Oh nice it works now. I wonder why it gave me an empty output before (even though the build log appeared to get all the dependencies and actually compile it). Nonetheless I've updated it so i think it ought to be good to go

Copy link
Contributor

doronbehar left a comment

Passes build and appears to be running as should.

@Enteee
Enteee approved these changes May 25, 2020
Copy link
Contributor

Enteee left a comment

looks good

@nixos-discourse
Copy link

nixos-discourse commented May 25, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/163

@Lassulus Lassulus merged commit ab3f7b6 into NixOS:master May 25, 2020
16 checks passed
16 checks passed
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="d5c52db"; rev="d5c52dbcce4f49a63eefba548a47b0e4722775ec"; } ./pkgs/t
Details
grahamcofborg-eval-lib-tests nix-build --arg pkgs import ./. {} ./lib/tests/release.nix
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d5c52db"; rev="d5c52dbcce4f49a63eefba548a47b0e4722775ec"; } ./nixos/
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d5c52db"; rev="d5c52dbcce4f49a63eefba548a47b0e4722775ec"; } ./nixos/
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d5c52db"; rev="d5c52dbcce4f49a63eefba548a47b0e4722775ec"; } ./nixos/
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d5c52db"; rev="d5c52dbcce4f49a63eefba548a47b0e4722775ec"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d5c52db"; rev="d5c52dbcce4f49a63eefba548a47b0e4722775ec"; } ./pkgs/t
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="d5c52db"; rev="d5c52dbcce4f49a63eefba548a47b0e4722775ec"; } ./pkgs/t
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
rmapi, rmapi.passthru.tests on aarch64-linux Success
Details
rmapi, rmapi.passthru.tests on x86_64-linux Success
Details
@Enteee Enteee mentioned this pull request May 26, 2020
1 of 4 tasks complete
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

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