geiger-rs / cargo-geiger

Detects usage of unsafe Rust in a Rust crate and its dependencies.
https://crates.io/crates/cargo-geiger
1.39k stars 65 forks source link

'features' flags does not work #20

Closed RazrFalcon closed 6 years ago

RazrFalcon commented 6 years ago

This one:

        --features <FEATURES>     Space-separated list of features to activate
        --all-features            Activate all available features
        --no-default-features     Do not activate the `default` feature
anderejd commented 6 years ago

Thanks for the report!

I actually did try them on your resvg crate and it seemed to me like it worked as expected. Can you elaborate a bit on "does not work"? :)

RazrFalcon commented 6 years ago

If you set --all-features it should pull pango, cairo, resvg-qt dependencies. But according to the log - it's not.

anderejd commented 6 years ago

I see the dependencies, pango, cairo and resvg-qt, they are scanned but "not used"...

...
0/4        0/879        0/2    0/0     0/75       ├── cairo-rs v0.4.1
...
0/0        0/1375       0/0    0/0     0/65       │   ├── pango v0.4.0
...
0/0        0/241        0/0    0/0     0/2        ├── resvg-qt v0.3.0 (https://github.com/RazrFalcon/resvg-qt?rev=fd0ed4f#fd0ed4f4)
...

Is this different from your results? I'm on macOS right now. I might add that I can't build or cargo check --all-features due to 'QT_DIR is not set: NotPresent'.

RazrFalcon commented 6 years ago

I have an error, so I don't know. According to the log in https://github.com/anderejd/cargo-geiger/issues/19, there are no pango, cairo, etc.

anderejd commented 6 years ago

Do you get the panic when running cargo geiger or cargo geiger --all-features from the crate root too, directly from resvg? (not from resvg/tools/rendersvg).

I suspect that this issue will need to wait until the error handling is sorted out the the panics are resolved.

RazrFalcon commented 6 years ago

Now I see it. Thanks.

RazrFalcon commented 6 years ago

But I still have function is never used warnings, which indicates that --all-features was ignored in the process. Maybe this is ok.

anderejd commented 6 years ago

But I still have function is never used warnings, which indicates that --all-features was ignored in the process. Maybe this is ok.

Yeah, I see those warnings here too. They seems to be identical to that warnings that show up when using cargo check or cargo build so that seems consistent with the normal cargo behavior at least.

I don't know if the warnings would go away on this system when adding --all-features since I get the QT_DIR missing error when trying that.

anderejd commented 6 years ago

which indicates that --all-features was ignored in the process.

Ah... Now I get it. Yes. it seems like the normal cargo is doing something different with --all-features. I suspect that the build scripts could be involved in this issue. I'll open new issue for build scripts and reference this issue.

anderejd commented 6 years ago

This error is reported by cargo check --all-features when running from the root in the resvg crate, on my system:

error: failed to run custom build command for `resvg-qt v0.3.0 (https://github.com/RazrFalcon/resvg-qt?rev=fd0ed4f#fd0ed4f4)`
process didn't exit successfully: `/Users/a/code/resvg/target/debug/build/resvg-qt-c1a47417ecc6dfb6/build-script-build` (exit code: 101)
--- stderr
thread 'main' panicked at 'QT_DIR is not set: NotPresent', libcore/result.rs:945:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

This is expected since this system doesn't have QT_DIR. However, when running cargo geiger --all-features the same error is expected to show up, but isn't. This indicates that either build scripts arn't run or that the --all-features mode has issues in cargo-geiger.

RazrFalcon commented 6 years ago

However, when running cargo geiger --all-features the same error is expected to show up, but isn't.

--all-features must enable qt-backend, which requires QT_DIR on macOS.

anderejd commented 6 years ago

--all-features must enable qt-backend, which requires QT_DIR on macOS

Yes, I did expect the same error to appear when running cargo geiger with --all-features, but there's no error for me, which makes me suspect that the build scripts are not triggered. Am I being confused? :)

RazrFalcon commented 6 years ago

My own cargo-bloat works correctly in this scenario, so I think that this is a geiger bug.

anderejd commented 6 years ago

My own cargo-bloat works correctly in this scenario, so I think that this is geiger bug.

Yes I agree, I think I'm just failing to express myself in the last ~3 comments. I opened a new issue for suspected build script bugs in cargo-geiger and referenced this discussion as background.

anderejd commented 6 years ago

The original issue was correct all along, the feature flags was not being used correctly. Fixed in master at: https://github.com/anderejd/cargo-geiger/commit/33771aa57bea55fa868e50d9745a20aacd82da72

Thanks for the assistance! :)

I still think the build scripts needs to be investigated, but for different reasons, to scan the generated code from bindgen for example.