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

gr-osmosdr: use system python #54848

Closed
wants to merge 1 commit into from
Closed

gr-osmosdr: use system python #54848

wants to merge 1 commit into from

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented May 17, 2020

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

brew audit --strict gr-osmosdr fails saying:

gr-osmosdr:
  * Packages have been installed for:
      Python 2.7
    but this formula depends on:
      Python 3.8
Error: 1 problem in 1 formula detected

It's worth noting that the formula installs successfully and passes the tests, just not the audit.

As far as I can tell, brew is using python3. The libexec directory contains vendor/lib/python3.8/site-packages/ and no Python 2.7 directory.

Cheetah seems to be the only package that is needed. It seems that Cheetah has been replaced with Cheetah3, and the changelog says that version 3.2.5 works with Python 3.8. Note that when I tried to install Cheetah with pip in a Python 3.8 virtual environment it didn't work, while installing Cheetah3 did.

I used poet to generate the updated resource block.


When installing, brew gives this warning (although it doesn't seem related to me):

Warning: gr-osmosdr dependency gcc was built with a different C++ standard
library (libstdc++ from clang). This may cause problems at runtime.

I based these changes off of pyside (#54755) as well as using urh and xdot as references. In those files, the xy = Language::Python.major_minor_version Formula["python@3.8"].opt_bin/"python3" line is used. I added that here as it looks like it will make updating versions in the future easier, although I'm not sure that will be necessary until Python 4 so this change might not be needed. It's worth noting that this change doesn't seem to affect the audit error.

@Bo98
Copy link
Member

Bo98 commented May 22, 2020

The audit is correct and formula is using Python 2.7. The module is installed to lib/python2.7 and everything in bin invokes /usr/bin/python2.7.

gnuradio uses Python 2.7 so this cannot use Python 3. The python dependency should be replaced with depends_on :macos # gnuradio uses Python 2.

@iMichka
Copy link
Member

iMichka commented May 22, 2020

Yes, you need to remove depends_on "python@3.8", and use system Python 2 on Mac.
We are working on a gnuradio migration in #49733, but this may take more time :/

@Rylan12 Rylan12 changed the title gr-osmosdr: migrate to python@3.8 gr-osmosdr: use system python May 22, 2020
@Rylan12
Copy link
Member Author

Rylan12 commented May 22, 2020

Makes sense. What does the :macos dependency mean? I assume there's more to it than using system python2.

@Bo98
Copy link
Member

Bo98 commented May 22, 2020

On macOS, nothing really. On Linux, it blocks installation. It's useful as a marker so we know what uses Python 2, and will help aid the future merge of homebrew-core and linuxbrew-core.

@Rylan12
Copy link
Member Author

Rylan12 commented May 22, 2020

Can Linux users install from homebrew-core? I just assumed they used linuxbrew-core. I'm not sure if you're saying it would block installation now or in the future when they are merged.

@iMichka
Copy link
Member

iMichka commented May 22, 2020

Can Linux users install from homebrew-core? I just assumed they used linuxbrew-core. I'm not sure if you're saying it would block installation now or in the future when they are merged.

They are using linuxbrew-core. But we are working on merging both repos; so that there is only homebrew-core in the future.

:macos will just prevent installing gr-osmosdr on Linux as we do not provide Python 2 anymore. But this won't change anything because due to gnuradio, Linux users could also not install it (as gnuradio is still stuck on a Python 2 only version right now).

If we wanted to be really strict we should drop gr-osmosdr as it is using a deprecated Python 2, but we give the Mac users a little bit more of time before Apple completely removes Python 2.

@Rylan12
Copy link
Member Author

Rylan12 commented May 22, 2020

Thanks, that makes sense.

@iMichka
Copy link
Member

iMichka commented May 22, 2020

Thanks @Rylan12 !

@BrewTestBot
Copy link
Member

🤖 A scheduled task has triggered a merge.

@Rylan12 Rylan12 deleted the gr-osmosdr-python38 branch May 22, 2020 17:10
@mikemorris mikemorris mentioned this pull request May 27, 2020
5 tasks
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.

5 participants