Add minification process by GuillaumeGomez · Pull Request #50632 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation14 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
For the record, here's the error:
Building rustdoc for stage1 (x86_64-apple-darwin)
Compiling pulldown-cmark v0.1.2
Compiling tempdir v0.3.7
Compiling minifier v0.0.4
Compiling rustdoc v0.0.0 (file:///Users/imperio/rust/rust/src/librustdoc)
Compiling rustdoc-tool v0.0.0 (file:///Users/imperio/rust/rust/src/tools/rustdoc)
Finished release [optimized] target(s) in 54.79 secs
dyld: Library not loaded: @rpath/libminifier-ff582a1ef3309921.dylib
Referenced from: /Users/imperio/rust/rust/build/x86_64-apple-darwin/stage1/bin/rustdoc
Reason: image not found
The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.
Click to expand the log.
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
InsecurePlatformWarning
/usr/local/lib/python2.7/dist-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
InsecurePlatformWarning
Downloading https://files.pythonhosted.org/packages/18/61/4e0f977cfe063188d73622a91cab8b8b409b662f422303fc687f362d941f/awscli-1.15.18-py2.py3-none-any.whl (1.3MB)
0% |▎ | 10kB 16.2MB/s eta 0:00:01
1% |▌ | 20kB 1.9MB/s eta 0:00:01
2% |▉ | 30kB 2.2MB/s eta 0:00:01
3% |█ | 40kB 2.0MB/s eta 0:00:01
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)
Old (and bad apparently) habit. Good catch, thanks a lot!
Fixed the initial issue, now just need to make the minification process work as expected. :)
[WIP] Add minification process Add minification process
| include_bytes!("static/main.js"), |
|---|
| enable_minification)?; |
| write_minify(cx.dst.join(&format!("settings{}.js", cx.shared.resource_suffix)), |
| include_bytes!("static/settings.js"), |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we minimize these two *.js files at build.rs of rustdoc?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather prefer that we don't. We could but then disabling minification would be a bit more complicated and I'm not sure it's worth it.
| @@ -1020,6 +1028,18 @@ fn write(dst: PathBuf, contents: &[u8]) -> Result<(), Error> { |
|---|
| Ok(try_err!(fs::write(&dst, contents), &dst)) |
| } |
| fn write_minify(dst: PathBuf, contents: &[u8], enable_minification: bool) -> Result<(), Error> { |
| if enable_minification { |
| if let Ok(s) = String::from_utf8(contents.to_vec()) { |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use include_str! instead of include_bytes! for the static files to avoid this UTF-8 check.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Seems good to me. What do you think, @ollie27?
Yeah this looks good.
For the std docs it might make more sense for the minification to be done by rustbuild so it can apply to the docs generated by mdBook as well but this seems like a reasonable start anyway.
For now it's experimental. If we have no issue for a given time, we might want to expand this to other tools as well. Thanks for your review!
@bors: r=ollie27
📌 Commit e2db0a5 has been approved by ollie27
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
and removed S-waiting-on-review
Status: Awaiting review from the assignee but also interested parties.
labels
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request
bors added a commit that referenced this pull request
Rollup of 11 pull requests
Successful merges:
- #49767 (Rewrite docs for
std::ptr) - #50399 (save-analysis: handle aliasing imports a bit more nicely)
- #50594 (Update the man page with additional --print options)
- #50613 (Migrate the toolstate update bot to rust-highfive)
- #50632 (Add minification process)
- #50685 (ci: Add Dockerfile for dist-sparc64-linux)
- #50691 (rustdoc: Add support for pub(restricted))
- #50712 (Improve eager type resolution error message)
- #50720 (Add “Examples” section header in f32/f64 doc comments.)
- #50733 (Hyperlink DOI against preferred resolver)
- #50745 (Uncapitalize "You")
Failed merges:
Labels
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.