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

salt: update cffi resource #68490

Closed
wants to merge 4 commits into from
Closed

Conversation

carlocab
Copy link
Member

@carlocab carlocab commented Jan 7, 2021

See #68463.

@carlocab carlocab added the CI-force-arm [DEPRECATED] Don't pass --skip-unbottled-arm to brew test-bot. label Jan 7, 2021
@BrewTestBot BrewTestBot added no ARM bottle Formula has no ARM bottle python Python use is a significant feature of the PR or issue labels Jan 7, 2021
@carlocab carlocab mentioned this pull request Jan 7, 2021
34 tasks
@fxcoudert
Copy link
Member

Collecting cffi==1.14.3 😢

@carlocab
Copy link
Member Author

carlocab commented Jan 7, 2021

@SMillerDev
Copy link
Member

This formula gives issues on every update. Are we sure we still want to ship this?

@carlocab
Copy link
Member Author

carlocab commented Jan 8, 2021

To be fair at the moment it's giving no greater issues than other formulae that aren't yet ARM-compatible. That said, I don't mind deprecate!/disable! if it starts to do more than that.

@carlocab
Copy link
Member Author

carlocab commented Jan 8, 2021

@bayandin do we still need to do an inreplace to use a custom cffi as mentioned in this comment? #62864 (comment)

@bayandin
Copy link
Member

bayandin commented Jan 8, 2021

@bayandin do we still need to do an inreplace to use a custom cffi as mentioned in this comment? #62864 (comment)

We do.
Currently all versions are listed in buildpath/"requirements/static/pkg/py#{xy}/darwin.txt". And it has cffi==1.14.3.

@carlocab
Copy link
Member Author

carlocab commented Jan 8, 2021

Ok, cool, thanks. That's not hard. I'll fix it in a moment. Though, if someone has the time to do it now, please feel free.

@bayandin
Copy link
Member

bayandin commented Jan 8, 2021

Feel involved in the creation of this mess with salt, sorry folks.

I'll have another look at how we can list all dependencies in the formula without pyobjc*

@carlocab
Copy link
Member Author

carlocab commented Jan 13, 2021

The inreplace does get it to build with cffi 1.14.4, but now it won't even pass a salt --version test:

==> brew test --verbose salt
==> FAILED
==> Testing salt
/usr/bin/sandbox-exec -f /private/tmp/homebrew20210112-5661-74v104.sb ruby -W0 -I $LOAD_PATH -- /opt/homebrew/Library/Homebrew/test.rb /opt/homebrew/Library/Taps/homebrew/homebrew-core/Formula/salt.rb --verbose
==> /opt/homebrew/Cellar/salt/3002.2_1/bin/salt --version
WARNING: /opt/homebrew/Cellar/python@3.9/3.9.1_6/Frameworks/Python.framework/Versions/3.9/Resources/Python.app/Contents/MacOS/Python is loading libcrypto in an unsafe way

Error: salt: failed
An exception occurred within a child process:
  Test::Unit::AssertionFailedError: <0> expected but was
<nil>.

It's only happening on ARM, though, so I guess it just really has deeper issues that make it incompatible

@bayandin
Copy link
Member

bayandin commented Jan 13, 2021

@carlocab I've listed all the resources, that we use in salt (generated by slightly modified brew update-python-resources command, and removed all pyobjc*). Hope you don't mind 😄

Will create a PR for update-python-resources to make it easier to support the formula later.

Also, added pkg-config dependency to fix the latest failure:

Installing collected packages: cffi
  Created temporary directory: /private/tmp/pip-record-1y3qeeeh
    Running setup.py install for cffi: started
    Running command /opt/homebrew/Cellar/salt/3002.2_1/libexec/bin/python3.9 -u -c 'import sys, setuptools, tokenize; sys.argv[0] = '"'"'/private/tmp/pip-req-build-zqrfmfe8/setup.py'"'"'; __file__='"'"'/private/tmp/pip-req-build-zqrfmfe8/setup.py'"'"';f=getattr(tokenize, '"'"'open'"'"', open)(__file__);code=f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' install --record /private/tmp/pip-record-1y3qeeeh/install-record.txt --single-version-externally-managed --compile --install-headers /opt/homebrew/Cellar/salt/3002.2_1/libexec/include/site/python3.9/cffi
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: /opt/homebrew/opt/pkg-config/bin/pkg-config: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: exec: /opt/homebrew/opt/pkg-config/bin/pkg-config: cannot execute: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: /opt/homebrew/opt/pkg-config/bin/pkg-config: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: exec: /opt/homebrew/opt/pkg-config/bin/pkg-config: cannot execute: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: /opt/homebrew/opt/pkg-config/bin/pkg-config: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: exec: /opt/homebrew/opt/pkg-config/bin/pkg-config: cannot execute: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: /opt/homebrew/opt/pkg-config/bin/pkg-config: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: exec: /opt/homebrew/opt/pkg-config/bin/pkg-config: cannot execute: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: /opt/homebrew/opt/pkg-config/bin/pkg-config: No such file or directory
    /opt/homebrew/Library/Homebrew/shims/mac/super/pkg-config: line 9: exec: /opt/homebrew/opt/pkg-config/bin/pkg-config: cannot execute: No such file or directory
...

@carlocab
Copy link
Member Author

That's weird; that wasn't there before I added depends_on "libffi"... https://github.com/Homebrew/homebrew-core/runs/1691913764

But, sure, I don't mind the additional changes. Thanks!

@carlocab
Copy link
Member Author

Still no luck, sadly

@bayandin
Copy link
Member

      File "/private/tmp/pip-build-env-mghh8j2q/overlay/lib/python3.9/site-packages/cffi/api.py", line 48, in __init__
        import _cffi_backend as backend
    ImportError: dlopen(/private/tmp/pip-build-env-mghh8j2q/overlay/lib/python3.9/site-packages/_cffi_backend.cpython-39-darwin.so, 2): Symbol not found: _ffi_prep_closure
      Referenced from: /private/tmp/pip-build-env-mghh8j2q/overlay/lib/python3.9/site-packages/_cffi_backend.cpython-39-darwin.so
      Expected in: flat namespace
     in /private/tmp/pip-build-env-mghh8j2q/overlay/lib/python3.9/site-packages/_cffi_backend.cpython-39-darwin.so
    Preparing wheel metadata: finished with status 'error'

That's weird; that wasn't there before I added depends_on "libffi"... https://github.com/Homebrew/homebrew-core/runs/1691913764

Actually, for most of the formulae, we use system libffi, probably we'd like to stick with uses_from_macos "libffi" here as well

@carlocab
Copy link
Member Author

Actually, for most of the formulae, we use system libffi, probably we'd like to stick with uses_from_macos "libffi" here as well

Yes, I would also prefer system libffi. I was just trying it to see if it would help...

@@ -18,61 +19,254 @@ class Salt < Formula
sha256 "bbc802eb5097a9f2d02b182215ca73eb37bdd841972e321bcaac219c2b530431" => :mojave
end

depends_on "pkg-config" => :build
Copy link
Member

@bayandin bayandin Jan 13, 2021

Choose a reason for hiding this comment

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

My bad

Suggested change
depends_on "pkg-config" => :build

@bayandin
Copy link
Member

I guess that it could be related to the previous huge problem with salt and libcrypto (saltstack/salt#55084) which was fixed by saltstack/salt#56958.

Could anyone check which one libcrypto salt is trying to load on Apple Silicon?

@BrewTestBot
Copy link
Member

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@BrewTestBot BrewTestBot added the stale No recent activity label Feb 3, 2021
@carlocab carlocab closed this Feb 5, 2021
@carlocab carlocab deleted the salt-cffi branch February 5, 2021 14:48
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Mar 8, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Mar 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-force-arm [DEPRECATED] Don't pass --skip-unbottled-arm to brew test-bot. no ARM bottle Formula has no ARM bottle outdated PR was locked due to age python Python use is a significant feature of the PR or issue stale No recent activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants