Simplify nix_file_deps checking #86
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 456bb28.
To check that dependencies of `nix_file_deps` are correctly declared,
we copy all declared dependencies into the external repository
directory. If any dependencies were missed, then nix evaluation will
fail.
To test that this really works, remove the
```
nix_file_deps = ["//tests:pkgname.nix"],
```
line in `@nix-file-deps-test`. I tried writing a failure test for this
using Skylibs `analysistest` framework, but that doesn't seem to work
for failures in external repositories.
Like #76, this likewise closes #74, but in a simpler way.
mboes
added a commit
that referenced
this pull request
Apr 12, 2020
Historically, the user was able to see the output of `nix-build` in real-time. This was useful, because building derivations can take a long time. Even if the binary cache can be used for all derivations, downloading can take a long time. Then #77 came along, which made `nix-build` quiet as a side-effect. Later, #86 subsumed #77 with a simpler implementation. However, people argued in #82 that being quiet is the right thing to do, because otherwise workspace rule output can garble the output of `bazel query` and break scripts. So we stuck to quiet output and this was called out in the v0.6 changelog. It turns out that the problem affects other rule authors too (see bazel-contrib/rules_nodejs#583). This led to bazelbuild/bazel#10611, which fixes the problem for everyone, and shipped in Bazel 3.0. Now, all workspace rule output goes to `stderr`, so scripts calling `bazel query` shouldn't be affected. Given this upstream change, my position is that being verbose is now the right thing to do. If #85 is implemented, then being verbose should remain the default, at least for users of Bazel 3.0 onwards. Because even small packages may have a large set of dependencies that must be downloaded first. It's nigh impossible for the author of the workspace file to anticipate the state of the user's cache.
mboes
added a commit
that referenced
this pull request
Apr 15, 2020
Historically, the user was able to see the output of `nix-build` in real-time. This was useful, because building derivations can take a long time. Even if the binary cache can be used for all derivations, downloading can take a long time. Then #77 came along, which made `nix-build` quiet as a side-effect. Later, #86 subsumed #77 with a simpler implementation. However, people argued in #82 that being quiet is the right thing to do, because otherwise workspace rule output can garble the output of `bazel query` and break scripts. So we stuck to quiet output and this was called out in the v0.6 changelog. It turns out that the problem affects other rule authors too (see bazel-contrib/rules_nodejs#583). This led to bazelbuild/bazel#10611, which fixes the problem for everyone, and shipped in Bazel 3.0. Now, all workspace rule output goes to `stderr`, so scripts calling `bazel query` shouldn't be affected. Given this upstream change, my position is that being verbose is now the right thing to do. If #85 is implemented, then being verbose should remain the default, at least for users of Bazel 3.0 onwards. Because even small packages may have a large set of dependencies that must be downloaded first. It's nigh impossible for the author of the workspace file to anticipate the state of the user's cache.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To check that dependencies of
nix_file_depsare correctly declared, wecopy all declared dependencies into the external repository directory. If
any dependencies were missed, then nix evaluation will fail.
To test that this really works, remove the
line in
@nix-file-deps-test. I tried writing a failure test for thisusing Skylibs
analysistestframework, but that doesn't seem to work forfailures in external repositories.
Like #76, this likewise closes #74, but in a simpler way.
Closes #84