Conversation
github-actions
bot
added
6.topic: nixos
|
The \n problem seems to be resolved by this:
|
dasJ
force-pushed
the
feat/rework-etc
branch
2 times, most recently
from
9c1e6d4 to
afc983c
Compare
ofborg
bot
added
10.rebuild-darwin: 0
nixos/modules/system/etc/make-etc.sh
Outdated
There was a problem hiding this comment.
This still seems a little hacky to me. How about doing this instead, I used this myself before too when ingesting json output:
jq '.[]' -c "$entriesPath" | while read -r line; do
src=$(jq -r '.source' <<< "$line")
target=$(jq -r '.target' <<< "$line")
# ...
The -c makes sure that each list entry is output on a separate line as JSON
There was a problem hiding this comment.
I had that idea as well but it's horribly slow in practice. It took like 10 seconds for my local system
There was a problem hiding this comment.
Oh damn, yeah I guess it's a whole bunch of jq calls..
Possibly @sh could be used in some way to make this a bit nicer
There was a problem hiding this comment.
I tried that. @ajs124 helped me like 2 hours to get it to work using @sh but we were unable to. The main issue is that you get \n-separated values which you put into something like while read -r source target …; do. This however breaks when your value contains \n since it's a literal newline and that results in the loop running twice for half an entry each
There was a problem hiding this comment.
Which could potentially be fixed with \0 separated output, but jq doesn't support that in any released version.
Edit: Nvm, you're already doing that with that hack.
There was a problem hiding this comment.
Alternative idea: What if we sidestep JSON and jq completely by just generating the shell script in Nix itself. So instead of a shell for loop, we have a lib.concatMapStringsSep or so. This will generate a fairly big script, but I don't see a problem with that.
There was a problem hiding this comment.
Won't this cause the same problems as svanderburg/node2nix#46 (comment)?
There was a problem hiding this comment.
I'd think the scales we are dealing with are a bunch smaller.
There was a problem hiding this comment.
On my systems the etc derivation is typically around 20k with 150-200 entries for the sources. So yes, that's a lot smaller.
This commit changes the data format from 5 arrays to one big JSON file which ensures special characters (like \n) don't break anything. For this to work, make-etc.sh needs jq and will create a big list from \0-separated values (see the comment in the file). Also this commit quotes all variables, sets useful shell options, and fixes all shellcheck issues. Additionally, we show all errors before exiting instead of exiting on the first error. And finally, rename source to src since source is a bash keyword. Closes NixOS#130935
|
Alternative implementation: #131102 |
4 participants
This commit changes the data format from 5 arrays to one big JSON file
which ensures special characters (like \n) don't break anything.
For this to work, make-etc.sh needs jq and will create a big list from
\0-separated values (see the comment in the file).
Also this commit quotes all variables, sets useful shell options,
and fixes all shellcheck issues. Additionally, we show all errors before
exiting instead of exiting on the first error. And finally, rename
source to src since source is a bash keyword.
Closes #130935
Closes #131102
Closes #108779
Closes #41241
Closes #41276
Motivation for this change
Things done
sandboxinnix.confon non-NixOS linux)simple.nixnix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)