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

Exclude unneeded files from built package #203

Merged
merged 2 commits into from
Jul 31, 2020
Merged

Exclude unneeded files from built package #203

merged 2 commits into from
Jul 31, 2020

Conversation

darnuria
Copy link
Contributor

@darnuria darnuria commented Jul 23, 2020

By default cargo publish all the content versionned by git.
following insight given by cargo diet I used an explicit approach
to exclude test dataset etc. from the final package.

May clause #202

Cargo.toml Outdated Show resolved Hide resolved
@darnuria
Copy link
Contributor Author

Seems like the files are included, look like a bug/edge case in the reporting of cargo-diet I fill a bug in the repository of the project because, the line suggested by cargo dietdo not have the same semantics in cargo itself than in the size analisys of cargo diet.

I will push soon something to avoid test folder in each sub-folder or at least in src it seems to work. (Need some test to be sure)

Cargo.toml Outdated Show resolved Hide resolved
@darnuria
Copy link
Contributor Author

darnuria commented Jul 24, 2020

I think it might be easier to just use exclude instead of include: https://doc.rust-lang.org/cargo/reference/manifest.html#the-exclude-and-include-fields

Just made a commit with explicit exclusion if it's okay, I can rebase it, if it not I can change the Pull-request.

Edit: Random-idea a possibility with exclusion is to just exclude big samples test file, it may need to change some test to need an option if test are desired in the final package.

Quick analysis of the size of the package

I checked the package size by doing:

cargo clean
cargo publish --dry-run
ls -al ./target/package

Results:

# Before sha c240586a31f19d06b4a82a2280e7735832aa0c80
78972 juil. 24 23:46 bson-1.0.0.crate
# After 
43993 juil. 24 23:33 bson-1.0.0.crate

Something like 34979 bytes saved!

Sorry for my English I am a not an English native writer and also thanks for your patience! :)

@darnuria
Copy link
Contributor Author

There was a effectively a bug in crago-dietabout the interpretation of globing in cargo manifest format: the-lean-crate/cargo-diet#6 seems solved.

@saghm
Copy link
Contributor

saghm commented Jul 31, 2020

Any chance you'd be able to rebase this branch with master? I think that should fix the lint failures, and then this should be review for the rest of the team to review!

darnuria added 2 commits July 31, 2020 20:08
By default cargo publish all the content versionned by git.
following insight given by cargo diet I used an explicit approach
to exclude test dataset etc. from the final package.
@darnuria
Copy link
Contributor Author

Hi, sure thanks for the follow-up!

I rebased it onto master, but I didn't squashed the two commit to keep information while reviewing of the first approach but if the exclusion one is selected, I will rebase my commits to keep just one.

@saghm
Copy link
Contributor

saghm commented Jul 31, 2020

Sounds good! I agree there's no reason to modify the commits you made until we're ready to merge.

I just authorized the CI run; once it passes, I'll mark this as ready for review and add the others!

@darnuria darnuria marked this pull request as ready for review July 31, 2020 19:24
@darnuria
Copy link
Contributor Author

Thanks I took the liberty to make it available for review since the result of the CI test were OK.

image

Copy link
Contributor

@patrickfreed patrickfreed left a comment

Choose a reason for hiding this comment

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

LGTM, and thanks for your contribution!

I think something like this would definitely be useful in the driver too, so I filed RUST-526 to track the work for that.

@saghm saghm changed the title Explicitly includes files for package publication. Exclude unneeded files from built package Jul 31, 2020
@saghm saghm merged commit fd6346c into mongodb:master Jul 31, 2020
@saghm
Copy link
Contributor

saghm commented Jul 31, 2020

Just merged; thanks for the PR!

@darnuria
Copy link
Contributor Author

darnuria commented Aug 1, 2020

Thanks for time and review! :)

@darnuria darnuria deleted the darnuria/cargo-include-list branch August 1, 2020 08:47
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.

4 participants