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

elmPackages.elm-coverage: init at 0.0.3 #82779

Merged
merged 1 commit into from Mar 17, 2020
Merged

Conversation

@turboMaCk
Copy link
Member

turboMaCk commented Mar 17, 2020

2/2 of elm-coverage packaging.

This indirectly depends on this fix of elm-test

  • 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.

cc @domenkozar

@@ -1,4 +1,4 @@
# This file has been generated by node2nix 1.8.0. Do not edit!
# This file has been generated by node2nix 1.7.0. Do not edit!

This comment has been minimized.

Copy link
@turboMaCk

turboMaCk Mar 17, 2020

Author Member

I'm a bit behind on my machine because I require Haskell's dbus package for my setup to work thus there is this downgrade.

This comment has been minimized.

Copy link
@domenkozar

domenkozar Mar 17, 2020

Member

node2nix should come from nixpkgs, but out of scope for this issue :)

This comment has been minimized.

Copy link
@turboMaCk

turboMaCk Mar 17, 2020

Author Member

Agree. I originally copied this script from some other place but as you can see it uses nix-shell -i bash -p nodePackages.node2nix so system's version of <nixpkgs>. I'm not sure how can this be fixed at the moment but it seems to me it would be better if this used node2nix from repository itself.

This comment has been minimized.

Copy link
@domenkozar

domenkozar Mar 17, 2020

Member

something like:

ROOT="$(realpath "$(dirname -- "$(readlink -f -- "${BASH_SOURCE[0]}")")"/../../../../..)"
$(nix-build $ROOT -A nodePacakges.node2nix --no-out-link)/bin/node2nix ...
# This should not be needed (thanks to binwrap* being nooped) but for some reason it still needs to be done
# in case of just this package
sed 's/\"install\".*/\"install\":\"echo no-op\",/g' --in-place node_modules/elmi-to-json/package.json

This comment has been minimized.

Copy link
@turboMaCk

turboMaCk Mar 17, 2020

Author Member

This is the biggest hack and I can't really explain why is needed.

This comment has been minimized.

Copy link
@domenkozar

domenkozar Mar 17, 2020

Member

Probably since it's needed for elm-test, which is installer here as a dependency?

This comment has been minimized.

Copy link
@turboMaCk

turboMaCk Mar 17, 2020

Author Member

That's the case of elm-verify-examples as well and in that case the writeScriptBin works as expected. I have a bad feeling that this somewhat depends on how PATH is being defined and potentially in case of this package the version from node_modules/.bin takes precedence over the script. That would be very unfortunate behavior. If that's the case we need a better way to patch this.

binwrap package also provides library functions which can (and are) called from packages code (and hopefully should just check the binaries are already present and do nothing). I think if we would try to avoid installing that package in any way we would need to do a lot of patching in source files. One thing that comes to mind is to let npm install the library but remove its bin files.

Some part of me always wants to start crying every time I start considering doing hacks like this but so far I don't have idea about how to solve these issues in more robust way.

This comment has been minimized.

Copy link
@domenkozar

domenkozar Mar 17, 2020

Member

Yeah most probably need to check $PATH precedence for node_modules, but I think this is good enough to work.

"elm-test",
"elm-upgrade",
"elm-verify-examples",
"elm-xref"

This comment has been minimized.

Copy link
@turboMaCk

turboMaCk Mar 17, 2020

Author Member

just a bit of sorting

@domenkozar domenkozar merged commit e6b5bcd into NixOS:master Mar 17, 2020
15 checks passed
15 checks passed
elmPackages.elm-coverage on aarch64-linux No attempt
Details
Evaluation Performance Report Evaluator Performance Report
Details
elmPackages.elm-coverage on x86_64-linux Success
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
@turboMaCk turboMaCk mentioned this pull request Mar 18, 2020
1 of 10 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

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