Skip to content

Fix: tensorflow empty config vars#49424

Merged
adamjstewart merged 3 commits intospack:developfrom
elfprince13:fix/tensorflow-empty-config-vars
Mar 12, 2025
Merged

Fix: tensorflow empty config vars#49424
adamjstewart merged 3 commits intospack:developfrom
elfprince13:fix/tensorflow-empty-config-vars

Conversation

@elfprince13
Copy link
Copy Markdown
Contributor

@elfprince13 elfprince13 commented Mar 11, 2025

Currently if Spack decides CC_OPT_FLAGS should be empty, this incorrectly drops Tensorflow's ./configure.py into interactive mode. Fix allows Tensorflow to distinguish between empty and not set.

Incidentally this patch is also pending at tensorflow/tensorflow#89032 so we can set an upper bound in package.py if that ever makes it into a release.

@elfprince13
Copy link
Copy Markdown
Contributor Author

@spackbot fix style

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Mar 11, 2025

Let me see if I can fix that for you!

@spackbot-app
Copy link
Copy Markdown

spackbot-app bot commented Mar 11, 2025

I was able to run spack style --fix for you!

spack style --fix
==> Running style checks on spack
  selected: import, isort, black, flake8, mypy
==> Modified files
  var/spack/repos/builtin/packages/py-tensorflow/package.py
==> Running import checks
  import checks were clean
==> Running isort checks
  isort checks were clean
==> Running black checks
reformatted var/spack/repos/builtin/packages/py-tensorflow/package.py
All done! ✨ 🍰 ✨
1 file reformatted.
  black checks were clean
==> Running flake8 checks
  flake8 checks were clean
==> Running mypy checks
Success: no issues found in 640 source files
  mypy checks were clean
==> spack style checks were clean
Keep in mind that I cannot fix your flake8 or mypy errors, so if you have any you'll need to fix them and update the pull request. If I was able to push to your branch, if you make further changes you will need to pull from your updated branch before pushing again.

I've updated the branch with style fixes.

Copy link
Copy Markdown
Member

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! Can you accept the CLA for your TF PR?

@adamjstewart adamjstewart enabled auto-merge (squash) March 12, 2025 10:48
@elfprince13
Copy link
Copy Markdown
Contributor Author

elfprince13 commented Mar 12, 2025 via email

@adamjstewart adamjstewart merged commit 8486a80 into spack:develop Mar 12, 2025
16 checks passed
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.

2 participants