Skip to content

Commit

Permalink
Revert "stdenv: Store one package per line in nix-support/propagated-*"
Browse files Browse the repository at this point in the history 
As @oxij points out in [1], this breakage is especially serious because
it changes the contents of built environments without a corresonding
change in their hashes. Also, the revert is easier than I thought.

This reverts commit 3cb745d.

[1]: #27427 (comment)
  • Loading branch information
@Ericson2314
Ericson2314 committed Jul 24, 2017
1 parent f4a8e6a commit b087618
Show file tree
Hide file tree
Showing 15 changed files with 22 additions and 31 deletions.
4 changes: 2 additions & 2 deletions pkgs/build-support/cc-wrapper/default.nix
View file
Expand Up @@ -275,9 +275,9 @@ stdenv.mkDerivation {
# Propagate the wrapped cc so that if you install the wrapper,
# you get tools like gcov, the manpages, etc. as well (including
# for binutils and Glibc).
printLines ${cc} ${cc.man or ""} ${binutils_bin} ${if libc == null then "" else libc_bin} > $out/nix-support/propagated-user-env-packages
echo ${cc} ${cc.man or ""} ${binutils_bin} ${if libc == null then "" else libc_bin} > $out/nix-support/propagated-user-env-packages
printLines ${toString extraPackages} > $out/nix-support/propagated-native-build-inputs
echo ${toString extraPackages} > $out/nix-support/propagated-native-build-inputs
''

+ optionalString (targetPlatform.isSunOS && nativePrefix != "") ''
Expand Down
2 changes: 1 addition & 1 deletion pkgs/build-support/gcc-wrapper-old/builder.sh
View file
Expand Up @@ -211,5 +211,5 @@ cp -p $utils $out/nix-support/utils.sh
# tools like gcov, the manpages, etc. as well (including for binutils
# and Glibc).
if test -z "$nativeTools"; then
printLines $gcc $binutils $libc $libc_bin > $out/nix-support/propagated-user-env-packages
echo $gcc $binutils $libc $libc_bin > $out/nix-support/propagated-user-env-packages
fi
2 changes: 1 addition & 1 deletion pkgs/build-support/setup-hooks/multiple-outputs.sh
View file
Expand Up @@ -202,7 +202,7 @@ _multioutPropagateDev() {

mkdir -p "${!propagaterOutput}"/nix-support
for output in $propagatedBuildOutputs; do
echo "${!output}" >> "${!propagaterOutput}"/nix-support/$propagatedBuildInputsFile
echo -n " ${!output}" >> "${!propagaterOutput}"/nix-support/$propagatedBuildInputsFile
done
}

2 changes: 1 addition & 1 deletion pkgs/build-support/trivial-builders.nix
View file
Expand Up @@ -84,7 +84,7 @@ rec {
mkdir -p $out/nix-support
cp ${script} $out/nix-support/setup-hook
'' + lib.optionalString (deps != []) ''
printLines ${toString deps} > $out/nix-support/propagated-native-build-inputs
echo ${toString deps} > $out/nix-support/propagated-native-build-inputs
'' + lib.optionalString (substitutions != {}) ''
substituteAll ${script} $out/nix-support/setup-hook
'');
Expand Down
2 changes: 1 addition & 1 deletion pkgs/desktops/kde-4.14/kde-package/default.nix
View file
Expand Up @@ -86,7 +86,7 @@ rec {
};})
''
mkdir -pv $out/nix-support
printLines ${toString list} | tee $out/nix-support/propagated-user-env-packages
echo "${toString list}" | tee $out/nix-support/propagated-user-env-packages
'';

# Given manifest module data, return the module
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/compilers/openjdk-darwin/8.nix
View file
Expand Up @@ -33,7 +33,7 @@ let
# any package that depends on the JRE has $CLASSPATH set up
# properly.
mkdir -p $out/nix-support
printLines ${setJavaClassPath} > $out/nix-support/propagated-native-build-inputs
echo -n "${setJavaClassPath}" > $out/nix-support/propagated-native-build-inputs
install_name_tool -change /usr/X11/lib/libfreetype.6.dylib ${freetype}/lib/libfreetype.6.dylib $out/jre/lib/libfontmanager.dylib
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/compilers/openjdk-darwin/default.nix
View file
Expand Up @@ -23,7 +23,7 @@ let
# any package that depends on the JRE has $CLASSPATH set up
# properly.
mkdir -p $out/nix-support
printLines ${setJavaClassPath} > $out/nix-support/propagated-native-build-inputs
echo -n "${setJavaClassPath}" > $out/nix-support/propagated-native-build-inputs
install_name_tool -change /usr/X11/lib/libfreetype.6.dylib ${freetype}/lib/libfreetype.6.dylib $out/jre/lib/libfontmanager.dylib
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/compilers/openjdk/7.nix
View file
Expand Up @@ -190,7 +190,7 @@ let
# any package that depends on the JRE has $CLASSPATH set up
# properly.
mkdir -p $jre/nix-support
printLines ${setJavaClassPath} > $jre/nix-support/propagated-native-build-inputs
echo -n "${setJavaClassPath}" > $jre/nix-support/propagated-native-build-inputs
# Set JAVA_HOME automatically.
mkdir -p $out/nix-support
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/compilers/openjdk/8.nix
View file
Expand Up @@ -202,7 +202,7 @@ let
# any package that depends on the JRE has $CLASSPATH set up
# properly.
mkdir -p $jre/nix-support
printLines ${setJavaClassPath} > $jre/nix-support/propagated-native-build-inputs
echo -n "${setJavaClassPath}" > $jre/nix-support/propagated-native-build-inputs
# Set JAVA_HOME automatically.
mkdir -p $out/nix-support
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/compilers/oraclejdk/jdk-linux-base.nix
View file
Expand Up @@ -165,7 +165,7 @@ let result = stdenv.mkDerivation rec {
ln -s $jrePath/lib/${architecture}/libnpjp2.so $jrePath/lib/${architecture}/plugins
mkdir -p $out/nix-support
printLines ${setJavaClassPath} > $out/nix-support/propagated-native-build-inputs
echo -n "${setJavaClassPath}" > $out/nix-support/propagated-native-build-inputs
# Set JAVA_HOME automatically.
cat <<EOF >> $out/nix-support/setup-hook
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/compilers/zulu/default.nix
View file
Expand Up @@ -54,7 +54,7 @@ in stdenv.mkDerivation rec {
find $out -name "*.so" -exec patchelf --set-rpath "$rpath" {} \;
mkdir -p $out/nix-support
printLines ${setJavaClassPath} > $out/nix-support/propagated-native-build-inputs
echo -n "${setJavaClassPath}" > $out/nix-support/propagated-native-build-inputs
# Set JAVA_HOME automatically.
cat <<EOF >> $out/nix-support/setup-hook
Expand Down
2 changes: 1 addition & 1 deletion pkgs/development/haskell-modules/generic-builder.nix
View file
Expand Up @@ -311,7 +311,7 @@ stdenv.mkDerivation ({
${optionalString isGhcjs ''
for exeDir in "$out/bin/"*.jsexe; do
exe="''${exeDir%.jsexe}"
printLines '#!${nodejs}/bin/node' > "$exe"
printf '%s\n' '#!${nodejs}/bin/node' > "$exe"
cat "$exeDir/all.js" >> "$exe"
chmod +x "$exe"
done
Expand Down
2 changes: 1 addition & 1 deletion pkgs/misc/misc.nix
View file
Expand Up @@ -23,7 +23,7 @@ in
*/
collection = {list, name} : runCommand "collection-${name}" {} ''
mkdir -p $out/nix-support
printLines ${builtins.toString list} > $out/nix-support/propagated-user-env-packages
echo ${builtins.toString list} > $out/nix-support/propagated-user-env-packages
'';

/* creates a derivation symlinking references C/C++ libs into one include and lib directory called $out/cdt-envs/${name}
Expand Down
5 changes: 2 additions & 3 deletions pkgs/stdenv/generic/builder.sh
View file
@@ -1,3 +1,4 @@
export PATH=
for i in $initialPath; do
if [ "$i" = / ]; then i=; fi
PATH=$PATH${PATH:+:}$i/bin
Expand All @@ -14,6 +15,4 @@ cat "$setup" >> $out/setup
# Allow the user to install stdenv using nix-env and get the packages
# in stdenv.
mkdir $out/nix-support
if [ "$propagatedUserEnvPkgs" ]; then
printf '%s\n' $propagatedUserEnvPkgs > $out/nix-support/propagated-user-env-packages
fi
echo $propagatedUserEnvPkgs > $out/nix-support/propagated-user-env-packages
20 changes: 6 additions & 14 deletions pkgs/stdenv/generic/setup.sh
View file
Expand Up @@ -215,11 +215,6 @@ isScript() {
if [[ "$magic" =~ \#! ]]; then return 0; else return 1; fi
}

# printf unfortunately will print a trailing newline regardless
printLines() {
[[ $# -gt 0 ]] || return 0
printf '%s\n' "$@"
}

######################################################################
# Initialisation.
Expand Down Expand Up @@ -305,12 +300,9 @@ findInputs() {
fi

if [ -f "$pkg/nix-support/$propagatedBuildInputsFile" ]; then
local fd pkgNext
exec {fd}<"$pkg/nix-support/$propagatedBuildInputsFile"
while IFS= read -r -u $fd pkgNext; do
findInputs "$pkgNext" $var $propagatedBuildInputsFile
for i in $(cat "$pkg/nix-support/$propagatedBuildInputsFile"); do
findInputs "$i" $var $propagatedBuildInputsFile
done
exec {fd}<&-
fi
}

Expand Down Expand Up @@ -802,17 +794,17 @@ fixupPhase() {
fi
if [ -n "$propagated" ]; then
mkdir -p "${!outputDev}/nix-support"
printLines $propagated > "${!outputDev}/nix-support/propagated-native-build-inputs"
echo "$propagated" > "${!outputDev}/nix-support/propagated-native-build-inputs"
fi
else
if [ -n "$propagatedBuildInputs" ]; then
mkdir -p "${!outputDev}/nix-support"
printLines $propagatedBuildInputs > "${!outputDev}/nix-support/propagated-build-inputs"
echo "$propagatedBuildInputs" > "${!outputDev}/nix-support/propagated-build-inputs"
fi

if [ -n "$propagatedNativeBuildInputs" ]; then
mkdir -p "${!outputDev}/nix-support"
printLines $propagatedNativeBuildInputs > "${!outputDev}/nix-support/propagated-native-build-inputs"
echo "$propagatedNativeBuildInputs" > "${!outputDev}/nix-support/propagated-native-build-inputs"
fi
fi

Expand All @@ -825,7 +817,7 @@ fixupPhase() {

if [ -n "$propagatedUserEnvPkgs" ]; then
mkdir -p "${!outputBin}/nix-support"
printLines $propagatedUserEnvPkgs > "${!outputBin}/nix-support/propagated-user-env-packages"
echo "$propagatedUserEnvPkgs" > "${!outputBin}/nix-support/propagated-user-env-packages"
fi

runHook postFixup
Expand Down

9 comments on commit b087618

@domenkozar
Copy link
Member

Choose a reason for hiding this comment

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

This should go through a staging branch

@Ericson2314
Copy link
Member Author

@Ericson2314 Ericson2314 commented on b087618 Jul 24, 2017 •
edited

Choose a reason for hiding this comment

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

You're probably right. That occurred to me shortly after I did the dead :/. OTOH reverting on staging would be much harder.

@joachifm
Copy link
Contributor

Choose a reason for hiding this comment

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

With the cc-wrapper changes in staging, looks like we'd end up with two mass rebuilds + one more once staging is merged. Is it possible to move the revert to staging?

@Ericson2314
Copy link
Member Author

@Ericson2314 Ericson2314 commented on b087618 Jul 24, 2017 •
edited

Choose a reason for hiding this comment

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

@joachifm the damage is done now that it's here? @copumpkin also wrote #27618 which targets master too. If we are too strained for resources we can cancel old evaluations.

@joachifm
Copy link
Contributor

Choose a reason for hiding this comment

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

Three mass rebuilds rather than 1 seems wasteful but whatever ...

@joachifm
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought it'd be possible to revert the revert on master and put it onto staging instead to save the work.

@copumpkin
Copy link
Member

@copumpkin copumpkin commented on b087618 Jul 24, 2017 •
edited

Choose a reason for hiding this comment

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

@joachifm I'm also considering putting a mass rebuild straight onto master, because the default Nix install (pointed at nixpkgs-unstable) is broken on several platforms right now: #27618

I could be persuaded to put it onto staging but I'd rather squeeze out another non-broken channel ASAP (especially when my understanding is that staging is a bit awkward right now and might take a while to merge back to master)

@joachifm
Copy link
Contributor

Choose a reason for hiding this comment

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

Are mass rebuilds on master faster to complete than staging? If not then I really fail to see the rationale for putting any of this on master. Anyway, I guess it's futile to argue so I've nothing further on this. I'll just resign to waiting :)

@Ericson2314
Copy link
Member Author

@Ericson2314 Ericson2314 commented on b087618 Jul 24, 2017 •
edited

Choose a reason for hiding this comment

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

@joachifm well it's always possible that staging broke something else which would delay the channel update. Undoing the breaking changes shouldn't be held up by other misc improvements.

Perhaps as @domenkozar implied (to me at least) with "a staging branch", we could have made a hydra job with a temp branch to cache things a bit further before hitting master. That would have been a more clear improvement, but also more work and a method I did not think of when I made the revert.

Please sign in to comment.