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

Support building on M1 macs #177

Closed
deeplow opened this issue Aug 4, 2022 · 5 comments · Fixed by #296
Closed

Support building on M1 macs #177

deeplow opened this issue Aug 4, 2022 · 5 comments · Fixed by #296
Assignees
Labels
Milestone

Comments

@deeplow
Copy link
Contributor

deeplow commented Aug 4, 2022

The shiboken2 package fails to install on m1s because it has no builds for ARM architectures on macbooks (no available wheels).


When running poetry install on an m1 macbook:

error_shiboken

Originally posted by @deeplow in #172 (comment)

@deeplow
Copy link
Contributor Author

deeplow commented Aug 4, 2022

Confirmed by @gmarmstrong in #161 (comment).

@deeplow deeplow changed the title Failed to setup dev env on m1 mac (shiboken2 not found) Support M1 macs Aug 5, 2022
@deeplow deeplow changed the title Support M1 macs Support building on M1 macs Aug 5, 2022
@deeplow
Copy link
Contributor Author

deeplow commented Aug 5, 2022

To clarify, Dangerzone runs fine on m1 macs. What I can't seem to do is to setup the development environment due to dependency issues (shiboken).

@deeplow
Copy link
Contributor Author

deeplow commented Nov 28, 2022

Until this this addressed, installing on M1 macs will involve activating rosetta (Apple's amd64 to arm emulation layer)

image(3)

I had not seen this before when setting up on an m1 mac but that was probably due to the machine being already provisioned with rosetta.

@apyrgio apyrgio added this to the 0.4.1 milestone Dec 13, 2022
@deeplow deeplow self-assigned this Dec 14, 2022
@deeplow
Copy link
Contributor Author

deeplow commented Dec 14, 2022

Our current approach to may be :

try: 
  Pyside2...
except ImportError:
  PySide6...

or maybe we should do with platform conditions.

On Poetry - find a way to have platform-specific dependencies - Add PySide6 for MacOS.

deeplow added a commit to deeplow/dangerzone that referenced this issue Dec 23, 2022
deeplow added a commit to deeplow/dangerzone that referenced this issue Jan 16, 2023
deeplow added a commit to deeplow/dangerzone that referenced this issue Jan 17, 2023
@apyrgio
Copy link
Contributor

apyrgio commented Jan 25, 2023

Update regarding the final implementation of this feature:

  1. PySide6 does not have sufficient type checking stubs, so we can't use Mypy to check it (see PySide6 does not offer sufficient Mypy hints #320). To circumvent this issue, we always perform type checking with the PySide2 stubs, since the API is compatible.

    • A problem with the existing PySide2 stubs is that they also bring PySide2 as a dependency. This means that in dev environments where we install PySide6 (currently Windows and MacOS), we would need to install PySide2 as well, which would potentially mask import problems. Worse, for MacOS M1 machines, this is not possible to do in the first place, since we can't install PySide2.

      A solution to this problem is to replace PySide2-stubs with types-PySide2, which does not have this dependency, and has better Mypy stubs.

  2. From now on, we will prefer PySide6 instead of PySide2, in systems that offer it. This means that we will try first importing PySide6, and then fallback to PySide2.

  3. The QRegExp class does not exist in PySide6. In their docs, they propose using QRegularExpression in Qt5, as a migration path towards Qt6. From a cursory glance on the sole regular expression that we use, we should not be affected by this change, so we will use this class.

apyrgio added a commit that referenced this issue Jan 25, 2023
Replace PySide2-stubs with types-PySide2, both of which are projects
that provide PySide2 typing hints, for the following reasons:

1. types-PySide2 is more complete and allows us to ditch some 'type:
   ignore' comments for Mypy.
2. PySide2-stubs also brings PySide2 as a dependency, which cannot be
   installed in MacOS M1 machines.

Refs #177
apyrgio added a commit that referenced this issue Jan 25, 2023
Replace PySide2-stubs with types-PySide2, both of which are projects
that provide PySide2 typing hints, for the following reasons:

1. types-PySide2 is more complete and allows us to ditch some 'type:
   ignore' comments for Mypy.
2. PySide2-stubs also brings PySide2 as a dependency, which cannot be
   installed in MacOS M1 machines.

Refs #177
apyrgio added a commit that referenced this issue Jan 25, 2023
Replace PySide2-stubs with types-PySide2, both of which are projects
that provide PySide2 typing hints, for the following reasons:

1. types-PySide2 is more complete and allows us to ditch some 'type:
   ignore' comments for Mypy.
2. PySide2-stubs also brings PySide2 as a dependency, which cannot be
   installed in MacOS M1 machines.

Refs #177
@apyrgio apyrgio closed this as completed Jan 30, 2023
@apyrgio apyrgio reopened this Jan 30, 2023
deeplow added a commit to deeplow/dangerzone that referenced this issue Jan 30, 2023
Replace PySide2-stubs with types-PySide2, both of which are projects
that provide PySide2 typing hints, for the following reasons:

1. types-PySide2 is more complete and allows us to ditch some 'type:
   ignore' comments for Mypy.
2. PySide2-stubs also brings PySide2 as a dependency, which cannot be
   installed in MacOS M1 machines.

Refs freedomofpress#177
deeplow added a commit to deeplow/dangerzone that referenced this issue Feb 8, 2023
Replace PySide2-stubs with types-PySide2, both of which are projects
that provide PySide2 typing hints, for the following reasons:

1. types-PySide2 is more complete and allows us to ditch some 'type:
   ignore' comments for Mypy.
2. PySide2-stubs also brings PySide2 as a dependency, which cannot be
   installed in MacOS M1 machines.

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

Successfully merging a pull request may close this issue.

2 participants