A small improvement here might be to use the PyPa Action instead: https://github.com/pypa/gh-action-pypi-publish

The big advantage is trusted publishing, instead of storing a password or token as a secret

that's cool, thanks for sharing!

@ahupp shall I make that change and you set it up on PyPI side?

Sounds like trusted publishing is the way to go. I recently got this email:

Hi ddelange!
Earlier this year, we announced that PyPI would require all users to enable a form of two-factor authentication on their accounts by the end of 2023.

Keeping your PyPI account secure is important to all of us. We encourage you to enable two-factor authentication on your PyPI account as soon as possible.

What forms of 2FA can I use?
We currently offer two main forms of 2FA for your account:

Security device including modern browsers (preferred) (e.g. Yubikey, Google Titan)
Authentication app (e.g. Google Authenticator)
Once one of these secure forms is enabled on your account, you will also need to use either Trusted Publishers (preferred) or API tokens to upload to PyPI.

What do I do if I lose my 2FA device?
As part of 2FA enrollment, you will receive one-time use recovery codes. One of them must be used to confirm receipt before 2FA is fully active. Keep these recovery codes safe - they are equivalent to your 2FA device. Should you lose access > to your 2FA device, use a recovery code to log in and swap your 2FA to a new device.

Read more aboutrecovery codes.

Why is PyPI requiring 2FA?
Keeping all users of PyPI is a shared responsibility we take seriously. Strong passwords combined with 2FA is a recognized secure practice for over a decade.

We are requiring 2FA to protect your account and the packages you upload, and to protect PyPI itself from malicious actors. The most damaging attacks are account takeover and malicious package upload.

To see this and other security events for your account, visit your account security history.

Read more on this blog post.

If you run into problems, read the FAQ page. If the solutions there are unable to resolve the issue, contact us via [email protected].

Thanks,
The PyPI Admins

4a715e2

@ahupp so the last thing for you to do is adding this repo as trusted publisher to https://pypi.org/manage/project/python-magic/settings/publishing/

Seems weird duplicating this logic between here and __init__.py, but I guess it's unavoidable since the __init__.py version is hidden inside Magic.__init__().

one way to avoid code duplication (and cyclic import) would be to add from magic.compat import magic_file on top of __init__.py, and use it as default argument:

Magic.__init__(magic_file=magic_file, ...)

option 2 would be to put the snippet in a new file and import it in both places (avoids cyclic import as well).

any preference here @ahupp?

add from magic.compat import magic_file on top of __init__.py,

I don't think that's a good idea, as it means that it'd be impossible to import any part of python-magic without also importing magic.compat. A compat module, specifically, should only be imported when needed.

The code could be moved to the top level of __init__.py, and imported into compat.py with from . import magic_file. I think that would be a better direction for things to flow, if going that route.

Oh, never mind. __init__.py already unconditionally imports magic.compat in its _add_compat() function. Which is always run on import.

It may be beneficial to add a library installation guide for SUSE as well

can you provide the relevant command?

fwiw, I think mostly all linux flavours will be covered by the wheels in the PR description, so those users won't be needing the install from source instructions provided here.

I guess that it would be
zypper install file-devel
it might as well be required to do (in vivo test is required though)
zypper install file-magic

Currently I don't have OpenSUSE at my disposal for tests and it's likely that it would be a default package. I don't promise anything, but I might find time soon-ish to test it.

Dev dependencies to use "make" and similar commands are not shipped by default in some distros, especially in those that are usually used by Docker. I'd suggest to ensure that necessary dependencies are installed first.

I wished set -euxo pipefail also effects commands inside function definitions. then we could remove && from every line here (and the subshell parentheses here and the || on L21), and we could test which make on top of this function or similar.

now, it simply falls back to install_precompiled (L69) when curl or make or compilation fails (error will be in output).

if make is not available, in the current setup we will only have done an unnecessary curl. not the worst but could be avoided.

any ideas?

I wished set -euxo pipefail also effects commands inside function definitions. then we could remove && from every line here (and the subshell parentheses here and the || on L21), and we could test which make on top of this function or similar.

any ideas?

It appears to be doing so already. Try this (and also the same script, but remove the first line or replace exit 1 to exit 0):

set -euxo pipefail
func(){
  echo 1
  (exit 1)
  echo 2
}

func

You made me think about other topic when you mentioned that curl can be executed. There is no cleanup on any step, it might be logical to make a ZIP (or gzip, doesn't matter) file cleanup from external function and do it regardless of the outcome. Same for windows. Pseudocode:

install_stuff(){
 real_install()
 rm potential_zip
}

Tried it on ubuntu's Docker, fails here :)
bash: curl: command not found

install_precompiled would work though

yes, the script is only intended for github actions default runner images and the cibuildwheel docker images (both have curl and make)

@estarfoo

Not a big deal, but if going with the piped curl output, personally I'd pair that with an explicit tar xvf -. Again, for the paranoid.