Skip to content

bazel-watcher: 0.5.0 -> 0.9.0 #51723

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

Merged
merged 1 commit into from
Jan 17, 2019
Merged

Conversation

uri-canva
Copy link
Contributor

@uri-canva uri-canva commented Dec 8, 2018

Motivation for this change

Upgrade bazel-watcher so that it can be built with newer versions of Bazel.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@uri-canva uri-canva force-pushed the bazel-watcher-0.9.0 branch from 49be769 to fb366a5 Compare December 8, 2018 10:21
@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 8, 2018
@uri-canva uri-canva mentioned this pull request Dec 8, 2018
10 tasks
@uri-canva
Copy link
Contributor Author

cc @kalbasit

Copy link
Member

@kalbasit kalbasit left a comment

Choose a reason for hiding this comment

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

I'm on vacation until the 26th with limited internet and no laptop. The code looks great, but I'd love to make sure it runs before approving. /cc @Mic92 if he's available to review.

@Mic92
Copy link
Member

Mic92 commented Dec 16, 2018

@GrahamcOfBorg build bazel-watcher

@Mic92
Copy link
Member

Mic92 commented Dec 16, 2018

I got locally:

 fixed-output derivation produced path '/nix/store/c2hzc13ajagwfdfl4qiafvl5fq046j0q-bazel-watcher-0.9.0-deps' with sha256 hash '03knj3jmn5nn36kf3m4s30b4ms7hhmd1k9rcdzy70s8q6v2y84yr' instead of the expected hash '11pimgaxfx0p003zdxjx9fc9whgxh7qmycihakki4hm2n9ys22fq'

@kalbasit
Copy link
Member

I got

fixed-output derivation produced path '/nix/store/jcnzfn13w02jn3lbf08pzq68jjz5grv7-bazel-watcher-0.9.0-deps' with sha256 hash '0239h6z605npjv8kpw3h3jkc0zlvbpvrr4xm30dvr9n5s9piqlnq' instead of the expected hash '11pimgaxfx0p003zdxjx9fc9whgxh7qmycihakki4hm2n9ys22fq'

I think the buildBazelPackage needs to be adjusted again. I uploaded /nix/store/jcnzfn13w02jn3lbf08pzq68jjz5grv7-bazel-watcher-0.9.0-deps here, can you compare the files with your version and adjust buildBazelPackage to remove those differences? See 86a5535 for my latest attempt at removing those differences.

@uri-canva
Copy link
Contributor Author

Sorry was on vacation too. Will take a look.

@uri-canva
Copy link
Contributor Author

Seems like the binaries include the output path in them, and strip -S doesn't strip it, so it's not a debug symbol. There some information in https://blog.filippo.io/reproducing-go-binaries-byte-by-byte/ about reproducible go binaries, it might be related to the build id.

$ go run gosym.go fetch_repo
/tmp/nix-build-bazel-watcher-0.9.0-deps.drv-0/output/external/bazel_gazelle_go_repository_tools/src/github.com/bazelbuild/bazel-gazelle/cmd/fetch_repo/fetch_repo.go
$ go run gosym.go ~/Downloads/nix/store/jcnzfn13w02jn3lbf08pzq68jjz5grv7-bazel-watcher-0.9.0-deps/bazel_gazelle_go_repository_tools/bin/fetch_repo
/build/output/external/bazel_gazelle_go_repository_tools/src/github.com/bazelbuild/bazel-gazelle/cmd/fetch_repo/fetch_repo.go

@uri-canva uri-canva mentioned this pull request Jan 16, 2019
10 tasks
@Mic92
Copy link
Member

Mic92 commented Jan 16, 2019

why is this a fixed-input derivation anyway?

@uri-canva
Copy link
Contributor Author

The whole derivation isn't fixed output, but the dependencies Bazel will download are. Unfortunately some of the dependencies it downloads also end up building native code.

@uri-canva
Copy link
Contributor Author

You just gave me an idea though.

@uri-canva uri-canva force-pushed the bazel-watcher-0.9.0 branch from 52c0fbb to 6c30139 Compare January 17, 2019 00:35
@uri-canva
Copy link
Contributor Author

Done, change it to delete the gazelle binary tools since they can be rebuilt during the build, because it doesn't require accessing the network.

@kalbasit
Copy link
Member

cool, let me give it another try! Thank you @uri-canva for working on this!

@kalbasit kalbasit merged commit cd29409 into NixOS:master Jan 17, 2019
@uri-canva uri-canva deleted the bazel-watcher-0.9.0 branch January 17, 2019 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants