Hi! I'm re-evaluating busybox patches once again, and once again come across this temp-deb-installer-hack.patch thing, and am really wonder if it is still needed.
And unlike the previous time, I actually looked at what is going on here. First, we now revert a commit in busybox, which is basically doing the same thing which our temp-d-i-hack patch does: commit 9c143ce52da11ec3d21a3491c3749841d3dc10f0 Author: Denys Vlasenko <[email protected]> Date: Thu Nov 2 12:56:24 2017 +0100 ash: retain envvars with bad names in initial environment. Closes 10231 Reworks "ash: [VAR] Sanitise environment variable names on entry" commit. The metadata of our revert is: From: Cyril Brulebois <[email protected]> Date: Fri, 29 Mar 2019 17:42:23 +0100 Subject: Revert 9c143ce52da11ec3d21a3491c3749841d3dc10f0 just to make sure the next patch can be applied And next we apply our version with FEATURE_DI_ENV_HACK. Initially Chris removed this FEATURE_DI_ENV_HACK change when preparing 1.30.1-1, as no longer needed, because the busybox commit above (9c143ce52d) does what's needed here. However, this introduced bug #925979 which later fixed by Cyril by reverting that commit in busybox and re- enabling the DI_ENV_HACK patch. And now we have quite some mess. The problem is that now, busybox ash does *not* export environment with invalid names, unlike, say, bash. Only the d-i version of busybox does this: $ env foo/bar=baz bash bash$ env | grep foo foo/bar=baz ^D $ env foo/bar=baz busybox ash ash$ env | grep foo ^D So, in the end, we *broke* exporting of environment with invalid names, while the intention was to keep it. Now, removing both the revert and the d-i patch hack, the new busybox ash behaves as expected: $ env foo/bar=baz ./busybox ash ash$ env | grep foo foo/bar=baz ^D Here's the actual difference in the exporting part, after applying both patches: @@ -14856,18 +14848,13 @@ init(void) initvar(); for (envp = environ; envp && *envp; envp++) { -/* Used to have - * p = endofname(*envp); - * if (p != *envp && *p == '=') { - * here to weed out badly-named variables, but this breaks - * scenarios where people do want them passed to children: - * import os - * os.environ["test-test"]="test" - * if os.fork() == 0: - * os.execv("ash", [ 'ash', '-c', 'eval $(export -p); echo OK' ]) # fixes this - * os.execv("ash", [ 'ash', '-c', 'env | grep test-test' ]) # breaks this - */ - if (strchr(*envp, '=')) { +#if ENABLE_FEATURE_DI_ENV_HACK + if (strchr(*envp, '=')) +#else + p = endofname(*envp); + if (p != *envp && *p == '=') +#endif + { setvareq(*envp, VEXPORT|VTEXTFIXED); } } so as you can see, in busybox it was always the code path which is now enabled by ENABLE_FEATURE_DI_ENV_HACK only. In regular busybox, this exporting - which is guarded by our ENABLE_FEATURE_DI_ENV_HACK - is always enabled. What the mess. So I really wonder where this bug #925979 come from. Now, by reverting the mentioned busybox commit, we change one more place, in showvars(), where - after reverting - only the "valid" names are shown, the rest are filtered. Cyrill, can you think how #925979 happened? How about trying d-i with busybox without these two patches applied? The regular upstream version should work just fine... Thanks, /mjt
