Skip to content
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

Update tensorflow (1.15.1) to work with glibc 2.3 and bazel_1 #81035

Merged
merged 1 commit into from Mar 2, 2020

Conversation

mjlbach
Copy link
Contributor

@mjlbach mjlbach commented Feb 25, 2020

Motivation for this change

Fixes #77771, blocked by #81033

Things done

Add patch to grpc to allow building tensorflow with glibc > 2.3.0

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@mjlbach
Copy link
Contributor Author

mjlbach commented Feb 25, 2020

@andir, for some reason the SHA doesn't mismatch when I try to build without CUDA. I had to update it after the glibc patch though. Edit: Fixed

@andir
Copy link
Member

andir commented Feb 25, 2020

@andir, for some reason the SHA doesn't mismatch when I try to build without CUDA. I had to update it after the glibc patch though.

You mean the fixed output hash doesn't mismatch? You will have to invalidate it manually (once).

@mjlbach
Copy link
Contributor Author

mjlbach commented Feb 26, 2020

I just tried building this with a new machine and for some reason it looks like the patch isn't being applied. Update: Fixed

@mjlbach
Copy link
Contributor Author

mjlbach commented Feb 26, 2020

Ok, this PR should be gtg pending merging of #81033 and allowing bazelBuild to select the variant of bazel to use.

@mjlbach
Copy link
Contributor Author

mjlbach commented Feb 26, 2020

@andir I updated build-bazel-package to take a specific version of bazel, defaults to bazel, but will updated to bazel_1 after the other PR is merged, not sure if this is the best way to go...

}:

args@{
name
, bazelVersion ? bazel
Copy link
Member

Choose a reason for hiding this comment

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

I think bazelVerson gives the wrong hint (is it a string?). How about going with just bazel?

I've done the same change in e5d8d31

I like sticking to just bazel as that is the package name and is more intuitive for users already knowing the standard .override { } mechanism.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, I think this now matches your implementation?

@mjlbach mjlbach force-pushed the fix_tensorflow_glibc branch 3 times, most recently from d429d9c to e98e8b5 Compare February 26, 2020 12:36
@mjlbach mjlbach force-pushed the fix_tensorflow_glibc branch 3 times, most recently from fd1b632 to 30f24e9 Compare March 1, 2020 11:06
@FRidh
Copy link
Member

FRidh commented Mar 2, 2020

please rebase now that bazel_1 is in.

@mjlbach
Copy link
Contributor Author

mjlbach commented Mar 2, 2020

@FRidh done.

@FRidh
Copy link
Member

FRidh commented Mar 2, 2020

Also, commit names need to follow our guidelines.

* Apply glibc 2.3 patch
* build tensorflow with bazel_1
* Bump openssl version to 1.1
@mjlbach
Copy link
Contributor Author

mjlbach commented Mar 2, 2020

@FRidh fixed? The version bump isn't the primary purpose of this PR, but ended up being necessary for the glibc patch to work.

@mjlbach mjlbach changed the title Update tensorflow to work with new glibc Update tensorflow (1.15.1) to work with glibc 2.3 and bazel_1 Mar 2, 2020
@FRidh FRidh merged commit 3429698 into NixOS:master Mar 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tensorflow: command line flags not recognized by Bazel 2.0
3 participants