cargo2nix icon indicating copy to clipboard operation
cargo2nix copied to clipboard

Fix DEP_* values sanitization when whitespace among them

Open litchipi opened this issue 2 years ago • 9 comments

Context: When compiling GDK3, the DEP_GTK3-BACKENDS value is broadway wayland x11 but as it's not correctly quoted, the resulting command is: env CC=<stuff> < ... > DEP_GTK3-BACKENDS=broadway wayland x11 Resulting into a: env: wayland not found This quick and dirty fix allows to quote the value properly. I didn't find any clever bash trick to make the code prettier than this ugly fix, advices welcome on this :-)

litchipi avatar Oct 03 '22 15:10 litchipi

I'm going to get out the bash bible before merging a quick & dirty fix, but thanks for diagnosing the issue. That was a tricky place to fail.

psionic-k avatar Oct 04 '22 02:10 psionic-k

I'm preparing a different commit for this. I want to point out that you should be able to perform some cleanup in the preInstall hook or postBuild hook, depending on when the correct information is available.

psionic-k avatar Oct 04 '22 06:10 psionic-k

https://github.com/cargo2nix/cargo2nix/pull/292/commits/beca5239f5179a4f9d15a11495760a73a5f3c38c Try this

psionic-k avatar Oct 04 '22 06:10 psionic-k

Your commit works perfectly ! :+1:

litchipi avatar Oct 05 '22 11:10 litchipi

Well bad news I don't know why it worked but it didn't work twice Apparently you have to use double quotes instead,

 single_quote_whitespaced() {
 if [[ $1 == *[[:space:]]* ]]; then
-    echo "'$1'"
+    echo '"$1"'
 else
     echo $1
 fi

Which now makes the name of the function not so good, but it works now :sweat_smile:

litchipi avatar Oct 05 '22 14:10 litchipi

That is actually really odd.

echo '"$1"' will just output "$1" literally.

This would only make sense if, at the time the flag is read, $1 is set and "$1" properly interpolates to the value. It's likely just picking up whatever $1 is at that point in the builder. Since reading dep keys is a function, $1 is likely unset or set to something benign.

You might need to shell into the offending drv and see what values are present and what's going into the actual output. https://github.com/cargo2nix/cargo2nix/tree/unstable#declarative-build-debugging-shell

It can be faster and more informative than print line debugging these behaviors.

psionic-k avatar Oct 05 '22 15:10 psionic-k

The problem is that echo "'$1'" creates an execution line like this:

env CC_x86_64-unknown-linux-gnu=<stuff> CXX_x86_64-unknown-linux-gnu=<stuff> CC_x86_64-unknown-linux-gnu=<stuff> CXX_x86_64-unknown-linux-gnu=<stuff> DEP_GDK-3_INCLUDE=<stuff> 'DEP_GDK-3_BACKENDS='\''broadway' wayland 'x11'\''' DEP__INCLUDE=<stuff> <path to>/cargo build --release --target x86_64-unknown-linux-gnu --no-default-features --message-format json-diagnostic-rendered-ansi

Which leaves wayland out of the quote, thus creating the env: 'wayland': No such file or directory

litchipi avatar Oct 06 '22 09:10 litchipi

My bad the problem is not the function you created, it's because it requires dep_keys_sanitized="$(echo ''${depKeys[@]})" in mkcrate-utils.sh

litchipi avatar Oct 06 '22 11:10 litchipi

That makes sense. I want to be sure and kill this 100% rather than just leaving some more edge cases for the next person. The problem would seem extremely likely to have a generic solution, consistency in the write-read round trip.

psionic-k avatar Oct 07 '22 03:10 psionic-k

Right now I'm running with the two commits from @acristoffers:

https://github.com/acristoffers/cargo2nix/commit/75aa46cf71163d923f09cd94e56710db75f42fe2 https://github.com/acristoffers/cargo2nix/commit/34e9a649dc29ffe4becf65dadfdf0baef36a8ac7

I prefer readarray since it's AFAIK the more predictable way to go in this case.

Need to circle back and see if https://github.com/cargo2nix/cargo2nix/commit/beca5239f5179a4f9d15a11495760a73a5f3c38cm is still necessary or even made it into maintenance branch.

Thanks for changes. Closing in favor of ongoing work on maintenance branch.

psionic-k avatar Oct 15 '23 16:10 psionic-k