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

feat: allow building with the system Lua #969

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Kladki
Copy link
Contributor

@Kladki Kladki commented Apr 28, 2024

As per #943 (comment)

I tried to use default-features = false in the deps of yazi-fm and yazi-cli and have vendored-lua to be default in the dependencies, but when I did that it would just build with vendored lua every time. This is the only method that I could get to actually work, sorry if it's not ideal. :(

@sxyazi
Copy link
Owner

sxyazi commented Apr 28, 2024

Is it possible to still keep each crate's default = [ "vendored-lua" ], but prevent them from being referenced in e.g. yazi-fm and yazi-cli using something like yazi-dds = { path = "../yazi-dds", version = "0.2.5", default-features = false }?

@Kladki
Copy link
Contributor Author

Kladki commented Apr 28, 2024

I can try, but that would likely require setting default-features = false in every Cargo.toml as some of the other yazi dependencies depend on other ones which use the ones which depend on mlua.

@Kladki
Copy link
Contributor Author

Kladki commented Apr 28, 2024

Done. Nvm forgot to add the defaults again No apparently they were already there, ready!

yazi-dds = { path = "../yazi-dds", version = "0.2.5" }
yazi-plugin = { path = "../yazi-plugin", version = "0.2.5" }
yazi-proxy = { path = "../yazi-proxy", version = "0.2.5" }
yazi-dds = { path = "../yazi-dds", version = "0.2.5", default-features = false }
Copy link
Owner

Choose a reason for hiding this comment

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

Why do we need to add default-features = false to yazi-core? yazi-core is not the entry point of the program, is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because that dependency would otherwise expect to have those features, and we cannot override that without making enabling that feature not the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cargo tree seems unreliable for these tests, so I just eza -l target/release and compare the sizes. Without this there is no size difference between the default and --no-default-features, but with this there is a difference.

Copy link
Owner

Choose a reason for hiding this comment

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

What command did you use for building?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cargo build with and without --release and with and without --no-default-features

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