-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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 #1157: Implement versioned library naming #1268
Conversation
This looks good to me so far. A few comments:
|
Thanks for reviewing! I'll update and resent later! |
Updated with all comments addressed. (except the last one) (Looks like github dislike rebasing of pull requests. All inline comments given to the previous pull request are no longer listed on this page. Or, do we prefer close old pull request and open new one in such case?) I've also removed the change about including hashes of dependencies from my previous pull request. Because there is not discrimination about direct and indirect dependencies in the current implementation. I would prefer only include hashes of direct dependent crates, and encoding hash and hashes of direct dependencies into crates metadata. There are a few other details I'm not clear of. So I think it's better to start a new series on that. One thing in particular is what should be hashed? According to comments about build_link_meta:
If version is not hashed, I wonder how can it be transited across the dependency DAG. |
Version is not hashed in order to permit backward compatible libraries to be upgraded without causing all dependant libraries to be recompiled. If the user wishes to force that they can change any extra link metadata tags. This is a compromise between the desire to control and accurately reflect dependency, and the desire to do upgrades in the field the way os vendors prefer, using major.minor style versioning. |
Another issue, for crates with different names and without extra meta data, they may end up the same hash. I think it would be good to use hash as a unique id when resolving crate dependencies. Actually, there are more than one FIXMEs about replacing the Is there any background about not including crate name in hash? |
Also updated build to install versioned libraries and added a few missing actions for `make clean`.
On Linux/Mac, I got a segmentation fault: (gdb) bt #0 0x00000000007519af in glue_take584 () rust-lang#1 0x00000000006d4bec in back::rpath::get_rpath_flags::_3899df2ca513c603 () rust-lang#2 0x00000000006c7655 in back::link::link_binary::_7afde00a9791031c () rust-lang#3 0x00000000007d3ff5 in driver::rustc::compile_input::thunk9212 () rust-lang#4 0x0000000000710f24 in driver::rustc::time::_3e691b2a4ba58aee () rust-lang#5 0x000000000071a79d in driver::rustc::compile_input::_7b4a41b87c18e034 () rust-lang#6 0x000000000072f0a9 in driver::rustc::main::_cd8b8c8185af3dee () rust-lang#7 0x000000000072f1ed in _rust_main () rust-lang#8 0x00007ffff7e6e146 in task_start_wrapper (a=<optimized out>) at ../src/rt/rust_task.cpp:176 The variable `output` or `out_filename` becomes (null) after the definition of `fn unlib`. Move the function defintion to the beginning seems prevent the crash on Linux.
This looks really good. |
Pushed to master, working through first snapshot now. Here's hoping! |
Appears to have stuck with a couple little hacks to make the snapshots load right. Further work on getting version numbers to make sense given OS conventions will be done on branch. Thanks, this is great! |
Cranelift 0.87 now follows its own documentation regarding shift amounts, and implicitly masks them if the arch requires it. [0] [0]: bytecodealliance/wasmtime@0508932
This reverts commit 156bda8. This breaks the mir_overflow_off rustc test: https://github.com/bjorn3/rustc_codegen_cranelift/runs/7971362755?check_suite_focus=true#step:7:2904
* docs: fix typos * docs: revert UB repetition because it's used in different contexts Co-authored-by: Adrian Palacios <73246657+adpaco-aws@users.noreply.github.com>
Green on all bots.
#1157