lib/sources: remove 2 usages of toString on a path which will be read using fileContents #199812
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…al0000000000000000000000005-source'' happens on lazy-trees branch of Nix (NixOS/nix#6530)
This was referenced Nov 6, 2022
ofborg
bot
added
10.rebuild-darwin: 0
|
Getting the following in remember to use NixOS/nix#6530 |
roberth
requested changes
Nov 6, 2022
… using fileContents It gives a warning on the lazy-trees branch of Nix (NixOS/nix#6530) "warning: applying 'toString' to path '...' and then accessing it is deprecated, at '...'" 'else toString (/. + "${base}/${path}");' at line 183 may still cause a warning but i don't know how to reach that codepath and test so im leaving it untouched changing it to 'else /. + "${base}/${path}";' caused this error ``` error: a string that refers to a store path cannot be appended to a path at /home/systems/nixpkgs/lib/sources.nix:183:20: 182| then path 183| else /. + "${base}/${path}"; | ^ 184| in if pathIsRegularFile path ```
Artturin
force-pushed
the
removeusagesoftostringonpath1
branch
from
4c7ddd2 to
8c1b019
Compare
those commands work on master |
This requires us to avoid the `tryEval` + `throw` combination, because throw is strict in its error message, and we don't want to drop our single clue when `commitIdFromGitRepo` is used incorrectly.
|
The remaining problem was the |
|
Is this PR supposed to solve the fact that |
Thats unrelated |
|
Ok thanks @Artturin . For reference I created a separate issue there NixOS/nix#7308 |
|
Thanks @roberth |
|
https://github.com/NixOS/nixpkgs/actions/runs/3494613825/jobs/5850544462 This PR seems to have broken nixpkgs manual build. https://hydra.nixos.org/build/199090592/nixlog/1 It's blocking nixpkgs-unstable. |
|
Right, bisected to ec8f8f6 in particular. I don't understand why this happens, at least for now. But if noone does very soon, we should revert. |
|
Well, removing the doc-comment for |
|
Apparently this is related to the combination of this new function not getting exported from the file, while still getting documented. It's actually not good to get nixpkgs docs for a function that can't be used (from outside this file), so we can either A. export it --- a/lib/sources.nix
+++ b/lib/sources.nix
@@ -281,2 +281,3 @@
commitIdFromGitRepo
+ commitIdFromGitRepoOrError
B. or not include it in the docs. It's nice to keep description though, so e.g. this experiment worked for me: # Get the commit id of a git repo.
#
# Returns `{ value = commitHash }` or `{ error = "... message ..." }`.
#
# Example: commitIdFromGitRepo <nixpkgs/.git> |
|
I cannot rebuild any of my systems since this PR was merged, had to revert to commit~1 to have working systems again. #201779 accurately represents my issue, but I'm unaware of my system being a particularly esoteric configuration. Shouldn't this be reverted? It's been 4 days and master is, at least to me, not possible to build: (I'm not pasting a full trace because the issue afaik covers it) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check commit messages
Description of changes
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes