New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
envoy: 1.3.0 -> 1.11.1 #69279
envoy: 1.3.0 -> 1.11.1 #69279
Conversation
@@ -76,6 +76,11 @@ in stdenv.mkDerivation (fBuildAttrs // { | |||
# For example, in Tensorflow-gpu build: | |||
# platforms -> NIX_BUILD_TOP/tmp/install/35282f5123611afa742331368e9ae529/_embedded_binaries/platforms | |||
find $bazelOut/external -maxdepth 1 -type l | while read symlink; do | |||
# There is a top level symlink that points to the source directory, we need to keep that | |||
target="$(readlink "$symlink")" | |||
if [[ "$target" == "$NIX_BUILD_TOP/source" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not all projects have source
directory. Interesting that it's needed, maybe we need to filter /tmp
symlinks instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it appears if they have external repositories that refer back to the root repository via @//path:name
or @envoy//path:name
.
Is the symlink removal here for symlinks to NIX_BUILD_TOP/tmp
or to /tmp
? Because if it's /tmp
then we can delete all symlinks that point to outside of NIX_BUILD_TOP
. But even if it points to NIX_BUILD_TOP/tmp
why is that a problem? As long as it points inside NIX_BUILD_TOP
rewriting the symlink the build part of the derivation should make it work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that there is a hash in the path that changes from run to run (see an example above). Maybe we should drop everything to $NIX_BUILD_TOP/tmp
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes I see. I would have expected --output_user_root
to not add the hash. I guess because we control which directory that is we can then drop just the symlinks that point to that directory. Maybe we can rename it while we're at it, tmp
didn't make it obvious that it was the output user root.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a few regressions, 6 of these fail on master (tensorflow-gpu has incorrect hash), but 19 fail on this PR
19 package failed to build:
libtensorflow python27Packages.baselines python27Packages.dm-sonnet python27Packages.edward python27Packages.graph_nets python27Packages.tensorflow python27Packages.tensorflow-probability python27Packages.tensorflowWithCuda python27Packages.tflearn python37Packages.baselines python37Packages.dm-sonnet python37Packages.edward python37Packages.graph_nets python37Packages.optuna python37Packages.rl-coach python37Packages.tensorflow python37Packages.tensorflow-probability python37Packages.tensorflowWithCuda python37Packages.tflearn
2 package were build:
bazel-watcher envoy
@uri-canva the old and broken Can you rebase this on top of latest master? |
Will do thanks! |
Still need to update the hashes of the derivations that use |
sed -e '/^FILE:@com_github_gperftools_gperftools/autom4te.cache.*/d' -i $bazelOut/external/\@*.marker | ||
''; | ||
|
||
sha256 = "1sq8qdbg8f94dbyc607xpq2yrhw61j3yxq6kggywp6fbbg89fh55"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to build this locally to verify that it works, and I'm getting a hash mismatch:
$ nix-build -f . -A envoy
...
INFO: All external dependencies fetched successfully.
Loading: 379 packages loaded
Loading: 379 packages loaded
Loading: 379 packages loaded
@nix { "action": "setPhase", "phase": "installPhase" }
installing
hash mismatch in fixed-output derivation '/nix/store/zr2djgq4yra1gyjvgdrmz4qjmfkl3x2m-envoy-1.11.1-deps':
wanted: sha256:1sq8qdbg8f94dbyc607xpq2yrhw61j3yxq6kggywp6fbbg89fh55
got: sha256:0xlgw90m8sk47c6mr561hh7gbczsxlc8cqc2yr8xkv6vgdzydmz5
This was after rebasing onto upstream master, before which I saw a different hash for got:
. Is it possible that not enough is being cleaned up here or somehow the rule isn't running?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible there's other files that need to be cleaned in there.
I'm not using envoy and it was removed on master so I'll drop this. I was only updating it to clean up |
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @cstrahan @abbradar @mboes