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

Jump to bottom

haskell: default name to "" when root isn't a path in developPackage #103063

Merged
merged 1 commit into from May 8, 2021
Merged

haskell: default name to "" when root isn't a path in developPackage #103063

merged 1 commit into from May 8, 2021

Conversation

expipiplus1
Copy link
Contributor

See #103062

I don't think the documentation on developPackage really needs updating, at the risk of making it too verbose.

Motivation for this change

This allows derivation outputs to be used as the root attribute in developPackage

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.

cdepillabout
cdepillabout reviewed Nov 7, 2020
View reviewed changes
pkgs/development/haskell-modules/make-package-set.nix
@@ -230,7 +230,7 @@ in package-set { inherit pkgs stdenv callPackage; } self // {
# build that package.
developPackage =
{ root
, name ? builtins.baseNameOf root
, name ? if builtins.typeOf root == "path" then builtins.baseNameOf root else ""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would you want this? root gets passed to callCabal2nix, so it looks like it has to be a path?

What bug are you trying to fix here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, email response doesn't work for review comments. My response is below

@expipiplus1
Copy link
Contributor Author

expipiplus1 commented Nov 7, 2020 via email

@expipiplus1
Copy link
Contributor Author

Is there a nice way to get the name of a store path (i.e. everything after the hash), perhaps that would be better than "". Either way that string is just cosmetic.

@peti peti force-pushed the haskell-updates branch 3 times, most recently from ab1e60c to 9ea5a41 Compare November 13, 2020 21:04
@expipiplus1
Copy link
Contributor Author

rebased

@expipiplus1
Copy link
Contributor Author

Is it possible to merge or reject this please?

@peti peti force-pushed the haskell-updates branch 4 times, most recently from e6cf312 to 2fc326d Compare December 4, 2020 19:54
@expipiplus1
Copy link
Contributor Author

rebased

@peti peti force-pushed the haskell-updates branch 3 times, most recently from 2c7ad11 to 0e5f7e8 Compare December 18, 2020 19:28
@peti peti force-pushed the haskell-updates branch 3 times, most recently from ba32886 to c9e5a3f Compare January 2, 2021 18:58
@peti peti force-pushed the haskell-updates branch 4 times, most recently from 4c606a9 to 3ae580b Compare February 12, 2021 19:36
@peti peti force-pushed the haskell-updates branch 3 times, most recently from 56fa6fc to f3cb253 Compare February 19, 2021 19:56
@maralorn maralorn closed this May 7, 2021
@maralorn maralorn deleted the branch NixOS:haskell-updates May 7, 2021 21:55
@maralorn maralorn reopened this May 7, 2021
@expipiplus1
Copy link
Contributor Author

expipiplus1 commented May 8, 2021 via email

@maralorn
Copy link
Member

maralorn commented May 8, 2021

I am sorry. I just a had blip where a browser plugin accidentally removed the haskell-updates branch. I haven‘t looked into the content of this PR but I will, if you are still on it.

@expipiplus1
Copy link
Contributor Author

I've rebased, would be good to have this in or out one way or the other :)

@maralorn
Copy link
Member

maralorn commented May 8, 2021

Yeah, so I have looked into this. And I am certainly not opposed. But I have to say I really like the idea of getting some kind of identifier from the store path. I will look into that.

@maralorn
Copy link
Member

maralorn commented May 8, 2021

Hm, we can handcraft that, but that would be no tiny amount of code and there is no convenience function that I can find.

@maralorn maralorn merged commit 5999d83 into NixOS:haskell-updates May 8, 2021
@expipiplus1
Copy link
Contributor Author

Thanks!

@expipiplus1 expipiplus1 deleted the joe-path branch May 8, 2021 10:45
@sternenseemann
Copy link
Member

The more cosmetic solution to this would be using something like this: https://cs.tvl.fyi/depot/-/blob/nix/utils/default.nix#L20-59. Depending on how relevant the name it may not be worth it to enter the territory of builtins.unsafe* in order to solve solve the problem.

@maralorn
Copy link
Member

maralorn commented May 8, 2021

Thanks!

Thanks, to you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout cdepillabout left review comments

@maralorn maralorn Awaiting requested review from maralorn maralorn is a code owner

@sternenseemann sternenseemann Awaiting requested review from sternenseemann sternenseemann is a code owner

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@expipiplus1 @maralorn @sternenseemann @cdepillabout @SuperSandro2000