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

Reduce package size #11

Closed
Yoric opened this issue Mar 9, 2018 · 10 comments
Closed

Reduce package size #11

Yoric opened this issue Mar 9, 2018 · 10 comments

Comments

@Yoric
Copy link
Contributor

Yoric commented Mar 9, 2018

I'm trying to land as part of Firefox a package that depends on webidl-rs. That's unfortunately not possible because of the following files:

  • 1.9M lalrpop-snap/src/newparser.rs
  • 1.9M lalrpop-snap/src/parser/lrgrammar.rs
  • 1.9M lalrpop/src/newparser.rs
  • 2.3M lalrpop/src/parser/lrgrammar.rs
  • 832K webidl/src/parser/grammar.rs
  • 177K webidl/tests/mozilla_webidl.zip

It would be great if we could find a way to get rid of these files.

For mozilla_webidl.zip and grammar.rs, I believe it's just a matter of adding an exclude in Cargo.toml. For lalrpop-*, I believe that more recent versions don't contain such huge files anymore.

@Yoric Yoric changed the title Repackaging without tests? Reduce package size Mar 9, 2018
Yoric added a commit to Yoric/webidl-rs that referenced this issue Mar 9, 2018
This patch:
- excludes `mozilla_webidls.zip` from the deployed package;
- upgrades lalrpop-* to 0.14.0, which has smaller files.
sgodwincs added a commit that referenced this issue Mar 9, 2018
Issue #11 - Reduce package size
@Yoric
Copy link
Contributor Author

Yoric commented Mar 14, 2018

Apparently, I made a mistake and this doesn't remove all the files I thought it would.

Blocked by lalrpop/lalrpop#333.

Also, for some reason, src/parser/grammar.rs is still included despite the exclude.

That one is blocked by lalrpop/lalrpop#280.

@sgodwincs sgodwincs reopened this Mar 14, 2018
@sgodwincs
Copy link
Owner

Are you sure grammar.rs is still included? It doesn't seem to be there from what I see.

@mbrubeck
Copy link
Contributor

I believe #13 fixes the cause of grammar.rs sometimes ending up in the vendor directory. This could happen when cargo build builds the file out of the Cargo cache ($HOME/.cargo/registrysrc). The build script would then put its output in that cached source directory because of lalrpop/lalrpop#280. Later runs of cargo vendor then copy the source from that directory, including the generated file.

@sgodwincs
Copy link
Owner

I'm going to keep this open until lalrpop/lalrpop#333 has been dealt with.

@sgodwincs sgodwincs reopened this Mar 14, 2018
@Yoric
Copy link
Contributor Author

Yoric commented Mar 22, 2018

Ok, at least a new version of lalrpop has just landed (0.15.0), which includes lalrpop/lalrpop#295. That should take care of some of the files.

@sgodwincs
Copy link
Owner

sgodwincs commented Mar 23, 2018

I'm currently trying to get a new version out, but there's a problem with the include! used. I opened an issue lalrpop/lalrpop#350.

@Yoric
Copy link
Contributor Author

Yoric commented Apr 5, 2018

Lalrpop is just issued a fixed version. @sgodwincs Could you give it a try?

Barring any accident, this is the last blocker for https://bugzilla.mozilla.org/show_bug.cgi?id=1437004, so I'm eager to land it :)

@sgodwincs
Copy link
Owner

@Yoric Version 0.6.0 has been released. The only change you should have to make is that the Parser struct has been removed, and you can just directly call the parse_string function.

Let me know if there are anymore problems, I'm going to close this for now.

@Yoric
Copy link
Contributor Author

Yoric commented Apr 6, 2018

As far as I can tell, this does the trick.

Thanks!

@Yoric
Copy link
Contributor Author

Yoric commented Oct 9, 2018

... and the problem seems to reappear with 0.7.0 :/ Investigating

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

No branches or pull requests

3 participants