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

fix: remove unused dependencies #4624

Merged
merged 1 commit into from Sep 7, 2022

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Sep 7, 2022

Description

  • removes unused dependencies found by cargo udeps
  • remove tari_common_types dep from tari_script

@sdbondi sdbondi force-pushed the cargo-udeps branch 2 times, most recently from a5add64 to 6263e02 Compare September 7, 2022 05:45

derivative = "2.2.0"
libtor = "46.9.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might fail on windows

Copy link
Member Author

@sdbondi sdbondi Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tari_libtor is now an optional dependency in the base node/console wallet - so is the problem when --all-features is used?

Copy link
Member Author

@sdbondi sdbondi Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an explicit compile failure for windows builds that use libtor - although I'm not sure that is a great idea as windows may be supported one day reverted the commit, I think it's ok as long as someone doesn't build for windows with the libtor feature enabled

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so the problem is that it's trying to build libtor on windows. Wonder how easy it would be to exclude that package from the build for windows e.g cargo build --workspace --exclude tari_libtor

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep that's why I had put it under [target.'cfg(unix)'.dependencies]

🤷

Copy link
Member Author

@sdbondi sdbondi Sep 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice hack :) we can do it that way, but having a crate that builds no code in it is a bit weird. I'd rather not include the crate in the build at all if it's not used, i.e. you cant compile tari_base_node and tari_console_wallet with --features libtor if using windows

cargo udeps sees it an a unused dependency, which is a little erroneous but, I'd ideally rather not compile the crate at all in the normal rust way i.e. with tari_libtor as an optional dependency

Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol thanks. I would have eventually gotten my PR, but this is quicker

@stringhandler stringhandler merged commit 058f492 into tari-project:development Sep 7, 2022
@sdbondi sdbondi deleted the cargo-udeps branch September 7, 2022 07:11
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.

None yet

3 participants