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

Continuing the drive to TF2 #110

Merged
merged 81 commits into from
Apr 12, 2021
Merged

Continuing the drive to TF2 #110

merged 81 commits into from
Apr 12, 2021

Conversation

h-vetinari
Copy link
Member

This is an attempt (likely a bit of a hail-mary) to continue #104 by @jschueller, which has grown some conflicts with master. I fixed them as best as I could.

If someone wants to collaborate on this PR, I'll happily grant them push-access to my fork of this feedstock.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari h-vetinari mentioned this pull request Jan 11, 2021
@xhochy
Copy link
Member

xhochy commented Apr 8, 2021

@conda-forge/tensorflow @h-vetinari @njzjz This is ready for a round of review. I uploaded Linux builds to my channel https://anaconda.org/uwe.korn/repo and will upload osx-* builds later today when they finish. In accordance with CFEP-03, here are the build logs:

@h-vetinari
Copy link
Member Author

@xhochy
Looks very good to me, thanks for taking on the lion's share of the work here! I have some cosmetic improvements that I'd like push (no semantic changes), is that fine for you? I could also open a PR on my fork against this PR, if you want to review first.

@jschueller
Copy link
Contributor

no py39 build ?

@xhochy
Copy link
Member

xhochy commented Apr 8, 2021

no py39 build ?

I would put that in a follow-up PR as this probably requires a bit more work. Upstream also doesn't support 3.9 as some of the dependencies don't seem to support it.

@xhochy
Copy link
Member

xhochy commented Apr 8, 2021

@xhochy
Looks very good to me, thanks for taking on the lion's share of the work here! I have some cosmetic improvements that I'd like push (no semantic changes), is that fine for you? I could also open a PR on my fork against this PR, if you want to review first.

Feel free to push, I can review the diff here.

Comment on lines +2 to +3
From: "Uwe L. Korn" <[email protected]>
Date: Tue, 6 Apr 2021 17:52:09 +0200
Copy link
Member Author

Choose a reason for hiding this comment

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

I like having this meta-information in patches; taken from the commits with which you added the "header-less" patches.

Comment on lines +2 to +4
From: "Uwe L. Korn" <[email protected]>
Date: Thu, 1 Apr 2021 12:43:34 +0200
Subject: [PATCH 5/5] osx-arm64
Copy link
Member Author

Choose a reason for hiding this comment

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

Combined both osx-arm64 patches, because there didn't seem to be a strong reason for them to be separate; message taken from the file-name of the patch

@xhochy
Copy link
Member

xhochy commented Apr 8, 2021

@h-vetinari Thanks, change look fine ✅

@xhochy
Copy link
Member

xhochy commented Apr 8, 2021

osx-arm64 builds are now also available with the corresponding log: https://gist.github.com/d5076de61632b25d5106219d63643066

@izahn
Copy link

izahn commented Apr 8, 2021

no py39 build ?

I would put that in a follow-up PR as this probably requires a bit more work. Upstream also doesn't support 3.9 as some of the dependencies don't seem to support it.

The pkgs/main channel has 3.9 variants for tensorflow 2.4.1, and I don't see any 3.9 specific work-arounds in https://github.com/AnacondaRecipes/tensorflow_recipes, so this might actually "just work". OTOH tensorflow/tensorflow#44485 is still open...

@xhochy
Copy link
Member

xhochy commented Apr 12, 2021

@conda-forge/core In accordance with CFEP-03, I have built the packages locally for commit ee305da and uploaded them to my channel: https://anaconda.org/uwe.korn/repo The channel solely contains the packages for this build, i.e. everything should be copied over. Please someone from core: upload them and merge the PR afterwards.

Build logs:

For reference: It takes ~48h to build the osx-64 packages in serial. This is really heavy, the build is roughly 1/3 of the time with lief/binary rewrite(?) being the majority.

@isuruf
Copy link
Member

isuruf commented Apr 12, 2021

I had a look and it looks like the correct CONDA_BUILD_SYSROOT was used.

Thanks for the PR. Since you have access to do upload and merge, please go ahead and upload to conda-forge/main. If you want me to do it, let me know.

@xhochy
Copy link
Member

xhochy commented Apr 12, 2021

Thanks! Copied and merged. 🎆

@h-vetinari
Copy link
Member Author

Awesome, thanks so much @xhochy! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.