robo9k / rust-magic Goto Github PK

Just like #5 the magic_check() and magic_compile() functions also allow to pass in a colon separated list of database file.

Seems like this is available as an unstable rustfmt option: https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=format_code_in_doc_comments#format_code_in_doc_comments

This is required for #48 and also would also help new contributors in general.
Right now it only mentions how contributions will be licensed and mentions the security policy.

The embedded magic-sys crate needs to be moved into its own Git(Hub) repository.

This would allow for better separate issues (tests, wiki, doc, homepage etc.) and independent reuse by others.

magic_load() allows to pass in NULL to load the default database or passing in a colon separated (e.g. /usr/share/misc/magic:~/magic/custom-db) list of database filenames.
The Rust API should allow those use cases.

One possibility would be using a Vec<Path> for the load() function where an empty vec means "load default database" and "load these databases" in the non-empty cases.
In that case the load_default() function should be removed.

The cargo doc mentions an exclude key, which should be pre-seeded with e.g. .gitignore.

Evaluate which files (e.g. .travis.yml) can and should be excluded from the magic crate.

Today I updated rustc to latest nightly: rustc 1.14.0-nightly (6dc035ed9 2016-10-15) and cookie.load stopped working with an error: (MagicError { desc: "could not find any valid magic files!" }).

cargo test in rust-magic also started giving this errors:

running 7 tests
test tests::load_default ... ok
test tests::version ... ok
test tests::buffer ... FAILED
test tests::file ... FAILED
test tests::load_multiple ... ok
test tests::load_one ... FAILED
test tests::file_error ... FAILED

failures:

---- tests::buffer stdout ----
    thread 'tests::buffer' panicked at 'assertion failed: cookie.load(&vec!("data/tests/db-python").as_slice()).is_ok()', src/lib.rs:346
stack backtrace:
   1:     0x55d1b24b1d5f - std::sys::backtrace::tracing::imp::write::h22f199c1dbb72ba2
   2:     0x55d1b24b5f2d - std::panicking::default_hook::{{closure}}::h9a389c462b6a22dd
   3:     0x55d1b24b4cf6 - std::panicking::default_hook::h852b4223c1c00c59
   4:     0x55d1b24b53d8 - std::panicking::rust_panic_with_hook::hcd9d05f53fa0dafc
   5:     0x55d1b2390f53 - std::panicking::begin_panic::hc03e2830c2c89a5f
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/libstd/panicking.rs:413
   6:     0x55d1b23961bc - magic::tests::buffer::hd787b0f6e599b41e
                        at /tmp/rust-magic/src/lib.rs:346
   7:     0x55d1b2433e6b - <F as test::FnBox<T>>::call_box::h2e1a44e400b440fe
   8:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
   9:     0x55d1b2428d8b - std::panicking::try::do_call::h979401182dac68cb
  10:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
  11:     0x55d1b242f20e - <F as alloc::boxed::FnBox<A>>::call_box::h207caa79b4e1f62e
  12:     0x55d1b24b3bd0 - std::sys::thread::Thread::new::thread_start::h50b05608a499d2b2
  13:     0x7fb5241ff463 - start_thread
  14:     0x7fb523d2b97e - __clone
  15:                0x0 - <unknown>

---- tests::file stdout ----
    thread 'tests::file' panicked at 'assertion failed: cookie.load(&vec!("data/tests/db-images-png")).is_ok()', src/lib.rs:330
stack backtrace:
   1:     0x55d1b24b1d5f - std::sys::backtrace::tracing::imp::write::h22f199c1dbb72ba2
   2:     0x55d1b24b5f2d - std::panicking::default_hook::{{closure}}::h9a389c462b6a22dd
   3:     0x55d1b24b4cf6 - std::panicking::default_hook::h852b4223c1c00c59
   4:     0x55d1b24b53d8 - std::panicking::rust_panic_with_hook::hcd9d05f53fa0dafc
   5:     0x55d1b2390f53 - std::panicking::begin_panic::hc03e2830c2c89a5f
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/libstd/panicking.rs:413
   6:     0x55d1b2395920 - magic::tests::file::h87983e8d7a406ecc
                        at /tmp/rust-magic/src/lib.rs:330
   7:     0x55d1b2433e6b - <F as test::FnBox<T>>::call_box::h2e1a44e400b440fe
   8:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
   9:     0x55d1b2428d8b - std::panicking::try::do_call::h979401182dac68cb
  10:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
  11:     0x55d1b242f20e - <F as alloc::boxed::FnBox<A>>::call_box::h207caa79b4e1f62e
  12:     0x55d1b24b3bd0 - std::sys::thread::Thread::new::thread_start::h50b05608a499d2b2
  13:     0x7fb5241ff463 - start_thread
  14:     0x7fb523d2b97e - __clone
  15:                0x0 - <unknown>

---- tests::load_one stdout ----
    thread 'tests::load_one' panicked at 'assertion failed: cookie.load(&vec!("data/tests/db-images-png")).is_ok()', src/lib.rs:374
stack backtrace:
   1:     0x55d1b24b1d5f - std::sys::backtrace::tracing::imp::write::h22f199c1dbb72ba2
   2:     0x55d1b24b5f2d - std::panicking::default_hook::{{closure}}::h9a389c462b6a22dd
   3:     0x55d1b24b4cf6 - std::panicking::default_hook::h852b4223c1c00c59
   4:     0x55d1b24b53d8 - std::panicking::rust_panic_with_hook::hcd9d05f53fa0dafc
   5:     0x55d1b2390f53 - std::panicking::begin_panic::hc03e2830c2c89a5f
                        at /buildslave/rust-buildbot/slave/nightly-dist-rustc-linux/build/obj/../src/libstd/panicking.rs:413
   6:     0x55d1b2396de2 - magic::tests::load_one::h09d64198d4561952
                        at /tmp/rust-magic/src/lib.rs:374
   7:     0x55d1b2433e6b - <F as test::FnBox<T>>::call_box::h2e1a44e400b440fe
   8:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
   9:     0x55d1b2428d8b - std::panicking::try::do_call::h979401182dac68cb
  10:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
  11:     0x55d1b242f20e - <F as alloc::boxed::FnBox<A>>::call_box::h207caa79b4e1f62e
  12:     0x55d1b24b3bd0 - std::sys::thread::Thread::new::thread_start::h50b05608a499d2b2
  13:     0x7fb5241ff463 - start_thread
  14:     0x7fb523d2b97e - __clone
  15:                0x0 - <unknown>

---- tests::file_error stdout ----
    thread 'tests::file_error' panicked at 'assertion failed: `(left == right)` (left: `"cannot stat `\' (No such file or directory)"`, right: `"cannot stat `non-existent_file.txt\' (No such file or directory)"`)', src/lib.rs:362
stack backtrace:
   1:     0x55d1b24b1d5f - std::sys::backtrace::tracing::imp::write::h22f199c1dbb72ba2
   2:     0x55d1b24b5f2d - std::panicking::default_hook::{{closure}}::h9a389c462b6a22dd
   3:     0x55d1b24b4cf6 - std::panicking::default_hook::h852b4223c1c00c59
   4:     0x55d1b24b53d8 - std::panicking::rust_panic_with_hook::hcd9d05f53fa0dafc
   5:     0x55d1b24b5272 - std::panicking::begin_panic::hf6c488cee66e7f17
   6:     0x55d1b24b51b0 - std::panicking::begin_panic_fmt::hb0a7126ee57cdd27
   7:     0x55d1b2396a05 - magic::tests::file_error::hac9fb34322f09335
                        at /tmp/rust-magic/src/lib.rs:362
   8:     0x55d1b2433e6b - <F as test::FnBox<T>>::call_box::h2e1a44e400b440fe
   9:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
  10:     0x55d1b2428d8b - std::panicking::try::do_call::h979401182dac68cb
  11:     0x55d1b24bd9f6 - __rust_maybe_catch_panic
  12:     0x55d1b242f20e - <F as alloc::boxed::FnBox<A>>::call_box::h207caa79b4e1f62e
  13:     0x55d1b24b3bd0 - std::sys::thread::Thread::new::thread_start::h50b05608a499d2b2
  14:     0x7fb5241ff463 - start_thread
  15:     0x7fb523d2b97e - __clone
  16:                0x0 - <unknown>


failures:
    tests::buffer
    tests::file
    tests::file_error
    tests::load_one

test result: FAILED. 3 passed; 4 failed; 0 ignored; 0 measured

I have no idea what caused this but everything is still working fine on old version of nightly and stable.

See https://bestpractices.coreinfrastructure.org/en

In the spirit of robo9k/rust-magic-sys#8 this repo here should be dual licensed under both the MIT and the Apache-2.0 licenses.

Previous license related issues: #1 #3 #12 #25 #41 #42 #44

Currently the license is only MIT, not Apache-2.0

Requirement for a relicense like this in future versions is that all contributors of the current files agree with the change.

However I went through git blame of all the files currently present in main (dc3dbc5) and determined that I, @robo9k , am the author of all files or non-whitespace / non-trivial lines in text files.
As such while IANAL I believe my agreement is sufficient for this license change.

Documentation for contributors should state that future contributions will be dual-licensed as described above.

It seems that the inherited data/tests/rust-logo-128x128-blk.png is from e.g. rust-lang/rust-www which seems to be licensed as CC-BY (see rust-lang/rust#11562).

Given the requirements of CC-BY it might be desirable to replace rust-logo-128x128-blk.png with another test file.

See https://github.com/ossf/scorecard

The README currently contains a brief explanation how to setup a cargo bin project using the magic crate.
This could be moved into its own magic-usage project, which people can check out and run.

A more complete usage example would be reimplementing file(1) in Rust.

See https://rust-lang.github.io/api-guidelines/about.html

It looks like by default no databases are included. Where can I find more? Specifically I am looking for something that has most image types, but googling around has yielded nothing.

Seems like #41 broke GitHub's parsing of the repo license.
According to https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/adding-a-license-to-a-repository they want a file named "LICENSE or LICENSE.md (with all caps)".

The README.md already contains a "license" section, but no SPDX id.
The Cargo.toml also includes a license field.

Apart from GitHub maybe some research can gather expectations from other tools, e.g. a LICENSE.spdx file seems to be used elsewhere.

This is useful for testing, because the std::result::Result::unwrap_err() method and the assert_eq!() macro both require the success value to implement Debug. This is not a breaking change.

Currently the libc crate is used for some FFI types. Since Rust version 1.64.0 those types are available in core::ffi without an additional crate.

This would bump the MSRV to 1.64.0

See robo9k/rust-magic-sys#31 for the same change.

Due to hansjorg/rust-ci#22 the docs at http://rust-ci.org/robo9k/rust-magic/doc/magic/ are not being updated on Travis builds.
Since it does not look like the issue is going to be fixed any time soon, alternatives are needed.

Right now the same README.md is used for the Git repo and the published crate. The GitHub Pages website only consists of rustdoc.

This is also required/helpful for some of #48

Commits 5821967 and c14fe45 silenty introduced the "MIT" license as required cargo metadata for both the magic-sys and magic crates. Meanwhile the latter commit also added the authors of the forked wunki/rust-magic and thestinger/rust-magic repositories.

This forked repository is now a few commits ahead, but it would be more correct to discuss the license with @wunki and @thestinger anyways (as well as discussing whether they'd like to be mentioned as authors).

While MIT is my preferred license, BSD or even GPL v2 / v3 would be fine, too.

Things to do once a decision is made:

• Add a LICENSE / COPYING file
• Specify license in each Cargo.toml
• Mention license in README

The current pattern of creating a Cookie instance works like this:

let cookie = Cookie::open(flags::NONE).ok().unwrap();
cookie.load(&Path::new("/usr/share/misc/magic"));
cookie.file(&Path::new("assets/rust-logo-128x128-blk.png")).ok().unwrap());

Calling open(), then load() is uncomfortable to use and very close to the C API.
Furthermore the arguments for both functions could have sane defaults and thus be omitted.

The builder pattern (cf. Rust Guidelines) would allow to provide defaults and creating the instace in one build() step.
Furthermore the builder could provide functions to incrementally add databases (see #5, except for the default one which would need to be added explicitly). Flags could also be added/ORed incrementally.

Using the builder could work like this:

let default = CookieBuilder::build();
let custom = CookieBuilder::database(&Path::new("~/magic/custom-db")).flag(flags::CHECK).databases(vec![&Path::new("/usr/share/misc/magic"), &Path::new("/usr/local/share/misc/magic")]).flags(vec![flags::ERROR, flags::RAW]).build();

I tried to use the rustdoc example provided with Rust (Stable) 1.8.0, and it failed because of #![feature] which isn't supported in Rust Stable. It would be useful to note that requirement in the README, similar to other libraries which need Nightly. Thank you.

$ cargo +stable clippy -- -W clippy::pedantic -W clippy::restriction -W clippy::cargo

This is intentionally not enabled by default, but might contain relevant findings nonetheless.

The tests should load() a custom magic database, since they rely on exact matches of the detected magic result.

The buffer() test fails on my local machine, since the default database of my installed libmagic version contains another description for Python scripts compared to the Travis CI build.

I'm not quite happy with the current error handling.
There's one MagicError enum for aynthing that can go wrong in the crate and one opaque FfiError for anything that can go wrong when calling libmagic.

I think each function should return a specific error enum that only has the cases that can go wrong in that particular function.
For API stability, maybe the cases should currently not expose more than std:error::Error.
LibmagicError::ApiViolation should rather be an assertion. There's not much the user can do in that case and it might even be unsafe to keep using libmagic in that case.

Some references:

• https://rust-lang.github.io/api-guidelines/interoperability.html#c-good-err (see #92)
• https://sabrinajewson.org/blog/errors
• https://blog.burntsushi.net/rust-error-handling/

For user convenvience, maybe the one magic "whatever" type should be available to convert into.

thiserror is pretty conventient for development, but adds to dependencies and compile time. Maybe it could be replaced with cargo expand code once the dust has settled.

As a follow-up to #128 there's even more things that could be visualized and linked to with a badge, but that are not available on shields.io, but might be possible with https://shields.io/badges/dynamic-json-badge :

• 
• 
• 
• https://deps.dev/cargo/magic/0.16.0/dependencies/graph
• https://libraries.io/cargo/magic is available, but has different opinion on e.g. "outdated"
• https://clearlydefined.io/ seems to be broken right now
• https://github.com/badges/awesome-badges
• https://github.com/DenverCoder1/custom-icon-badges
• 
  
  
  
• 
• 
• https://shields.io/badges/oss-lifecycle
• https://ossindex.sonatype.org/component/pkg:cargo/magic
• 
• https://forthebadge.com/
  

While the crate itself has a version, other places are lacking proper versioning:

• GitHub releases
• Git tags
• Versioning strategy (especially once there's a v1.0.0)
  • Changelog
• A version() -> String function for the magic crate itself
• Version requirements for libmagic
  • A libmagic_version() function, possibly in magic-sys
• Version in rustdoc (cf. JavaDoc's @since)
• Version/milestones in issues

Currently most things are scoped to "cookie", but in the root and not a cookie module.
CookieFlags etc. could just be cookie::Flags.

When passing NULL as filename to magic_file() it will read from stdin instead.
The crate should support that usage.