Skip to content

remove python-magic dependency - #50

Open
cdleonard wants to merge 4 commits into
lackhove:mainfrom
cdleonard:feature/remove_magic
Open

cdleonard wants to merge 4 commits into
lackhove:mainfrom
cdleonard:feature/remove_magic

Conversation

@cdleonard

Copy link
Copy Markdown
Contributor

The python-magic package depends on the native libmagic library. This dependency needs to be added in containers and such.

Replace import magic with manual checks based on filename extension and header.

If people have trouble with auto-detect logic the best bet would be to add a shebang, even for non-executable library files. This is also used by shellcheck.

@lackhove

Copy link
Copy Markdown
Owner

I know libmagic is annoying to use in containers, but especially for detecting shell scripts, its very advanced.

Shipping our own implementation might be a step back and still opens a can of worms from a maintenance POV. In an other project, i moved to puremagic and never had any issues. This was for detecting media files, though, which is much simpler. Did you give that a try?

@cdleonard
cdleonard force-pushed the feature/remove_magic branch 2 times, most recently from d399f64 to cc91863 Compare May 25, 2026 12:18
@cdleonard

Copy link
Copy Markdown
Contributor Author

It makes sense to check puremagic

@cdleonard

Copy link
Copy Markdown
Contributor Author

I checked and puremagic does not currently support recognizing shell scripts at all - they're all reported as "plain text".

In theory shell script support could be added. But the puremagic 2.* branch also dropped support for python<3.12 which coverage-sh support.

I think carrying the code to recognize shell scripts inside coverage-sh itself is not unreasonable - it's a small amount of code and tightly connected to functionality.

@cdleonard
cdleonard force-pushed the feature/remove_magic branch from cc91863 to fce8cc5 Compare May 26, 2026 17:25
@cdleonard

Copy link
Copy Markdown
Contributor Author

Cleaned up some of the AI slop in the first version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants