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

Do not lose or reorder user-provided linker arguments #70665

Merged
merged 1 commit into from
Apr 6, 2020

Conversation

petrochenkov
Copy link
Contributor

Linker arguments are potentially order-dependent, so the order in which -C link-arg and -C link-args options are passed to rustc should be preserved when they are passed further to the linker.

Also, multiple -C link-args options are now appended to each other rather than overwrite each other.

In other words, -C link-arg=a -C link-args="b c" -C link-args="d e" -C link-arg=f is now passed as "a" "b" "c" "d" "e" "f" and not as "d" "e" "a" "f".

Addresses #70505 (comment).

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 1, 2020
@petrochenkov
Copy link
Contributor Author

It seems like we always keep the passed options precisely in (Codegen|Debugging)Options, e.g. Option<bool> with None if the option is not passed even if the option has a default and could be just bool.

In this PR it would be more convenient to normalize to Vec<String> immediately, then we wouldn't need LinkArgs(Component) at all.
I implemented the precise tracking for now even if only the normalized (flattened) version is used.
Not sure whether it's reasonable or not.

@petrochenkov
Copy link
Contributor Author

It seems like we always keep the passed options precisely

With #70729 this rule is no longer true, so I think I'll convert LinkArgs to a simple Vec<String>.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 3, 2020
@bors
Copy link
Contributor

bors commented Apr 4, 2020

☔ The latest upstream changes (presumably #69718) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Contributor Author

Updated.
r? @nagisa

@rust-highfive rust-highfive assigned nagisa and unassigned cramertj Apr 4, 2020
Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

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

r=me once we also have a test verifying sanity of handling of pre-link-arg(s), and maybe possibly some combination of pre-link-arg(s) and link-arg(s)?

If you don’t think one is necessary, that is also fine, in that case feel free to just r=me this.

@petrochenkov
Copy link
Contributor Author

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Apr 4, 2020

📌 Commit 13bd25e has been approved by nagisa

@bors
Copy link
Contributor

bors commented Apr 4, 2020

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2020
@rust-highfive
Copy link
Collaborator

The job mingw-check of your PR failed (pretty log, 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.
2020-04-04T19:10:33.4207697Z ========================== Starting Command Output ===========================
2020-04-04T19:10:33.4212559Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/84faf737-26a0-4f39-84b3-885285edd184.sh
2020-04-04T19:10:33.4213142Z 
2020-04-04T19:10:33.4217589Z ##[section]Finishing: Disable git automatic line ending conversion
2020-04-04T19:10:33.4248642Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/70665/merge to s
2020-04-04T19:10:33.4253977Z Task         : Get sources
2020-04-04T19:10:33.4254572Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-04-04T19:10:33.4254888Z Version      : 1.0.0
2020-04-04T19:10:33.4255091Z Author       : Microsoft
---
2020-04-04T19:10:35.8072002Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-04-04T19:10:35.8078240Z ##[command]git config gc.auto 0
2020-04-04T19:10:35.8081826Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-04-04T19:10:35.8084571Z ##[command]git config --get-all http.proxy
2020-04-04T19:10:35.8092100Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/70665/merge:refs/remotes/pull/70665/merge
---
2020-04-04T19:14:29.8928325Z  ---> 3fc1b512c57b
2020-04-04T19:14:29.8928833Z Step 6/7 : ENV RUN_CHECK_WITH_PARALLEL_QUERIES 1
2020-04-04T19:14:29.8930910Z  ---> Using cache
2020-04-04T19:14:29.8931765Z  ---> 5ee4295733f4
2020-04-04T19:14:29.8933570Z Step 7/7 : ENV SCRIPT python2.7 ../x.py test src/tools/expand-yaml-anchors &&            python2.7 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu &&            python2.7 ../x.py build --stage 0 src/tools/build-manifest &&            python2.7 ../x.py test --stage 0 src/tools/compiletest &&            python2.7 ../x.py test src/tools/tidy &&            /scripts/validate-toolstate.sh
2020-04-04T19:14:29.8937576Z  ---> 3d07a0fa42fe
2020-04-04T19:14:29.8971644Z Successfully built 3d07a0fa42fe
2020-04-04T19:14:29.9007726Z Successfully tagged rust-ci:latest
2020-04-04T19:14:29.9278762Z Built container sha256:3d07a0fa42feb5754fc13bb2f7010ebe13e4b8b8cdbebed0c75d8da320c8c8ad
2020-04-04T19:14:29.9278762Z Built container sha256:3d07a0fa42feb5754fc13bb2f7010ebe13e4b8b8cdbebed0c75d8da320c8c8ad
2020-04-04T19:14:29.9295466Z Looks like docker image is the same as before, not uploading
2020-04-04T19:14:36.0169240Z [CI_JOB_NAME=mingw-check]
2020-04-04T19:14:36.0486406Z [CI_JOB_NAME=mingw-check]
2020-04-04T19:14:36.0519958Z == clock drift check ==
2020-04-04T19:14:36.0528025Z   local time: Sat Apr  4 19:14:36 UTC 2020
2020-04-04T19:14:36.2152517Z   network time: Sat, 04 Apr 2020 19:14:36 GMT
2020-04-04T19:14:36.2177449Z Starting sccache server...
2020-04-04T19:14:36.3068904Z configure: processing command line
2020-04-04T19:14:36.3077404Z configure: 
2020-04-04T19:14:36.3096198Z configure: rust.parallel-compiler := True
---
2020-04-04T19:18:28.9366379Z     Checking rustc_span v0.0.0 (/checkout/src/librustc_span)
2020-04-04T19:18:33.8402916Z     Checking rustc_errors v0.0.0 (/checkout/src/librustc_errors)
2020-04-04T19:18:35.1206206Z     Checking rustc_feature v0.0.0 (/checkout/src/librustc_feature)
2020-04-04T19:18:35.1874506Z     Checking fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
2020-04-04T19:18:35.3909752Z     Checking rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
2020-04-04T19:18:36.2402919Z     Checking rustc_hir v0.0.0 (/checkout/src/librustc_hir)
2020-04-04T19:18:36.2898611Z     Checking rustc_session v0.0.0 (/checkout/src/librustc_session)
2020-04-04T19:18:37.9394619Z     Checking rustc_attr v0.0.0 (/checkout/src/librustc_attr)
2020-04-04T19:18:38.4480911Z     Checking rustc_parse v0.0.0 (/checkout/src/librustc_parse)
---
2020-04-04T19:20:39.2002449Z configure: build.locked-deps    := True
2020-04-04T19:20:39.2002959Z configure: llvm.ccache          := sccache
2020-04-04T19:20:39.2003653Z configure: build.cargo-native-static := True
2020-04-04T19:20:39.2004386Z configure: dist.missing-tools   := True
2020-04-04T19:20:39.2005226Z configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
2020-04-04T19:20:39.2006333Z configure: writing `config.toml` in current directory
2020-04-04T19:20:39.2006885Z configure: 
2020-04-04T19:20:39.2007643Z configure: run `python /checkout/x.py --help`
2020-04-04T19:20:39.2007910Z configure: 
---
2020-04-04T19:22:10.2114725Z Hugepagesize:       2048 kB
2020-04-04T19:22:10.2114973Z DirectMap4k:      155584 kB
2020-04-04T19:22:10.2115237Z DirectMap2M:     4038656 kB
2020-04-04T19:22:10.2115485Z DirectMap1G:     5242880 kB
2020-04-04T19:22:10.2179096Z + python2.7 ../x.py test src/tools/expand-yaml-anchors
2020-04-04T19:22:11.6162672Z Ensuring the YAML anchors in the GitHub Actions config were expanded
2020-04-04T19:22:11.6162672Z Ensuring the YAML anchors in the GitHub Actions config were expanded
2020-04-04T19:22:11.6170341Z Building stage0 tool expand-yaml-anchors (x86_64-unknown-linux-gnu)
2020-04-04T19:22:11.8838075Z    Compiling unicode-xid v0.2.0
2020-04-04T19:22:12.0258191Z    Compiling syn v1.0.11
2020-04-04T19:22:12.9815839Z    Compiling linked-hash-map v0.5.2
2020-04-04T19:22:12.9979887Z    Compiling lazy_static v1.4.0
2020-04-04T19:22:12.9979887Z    Compiling lazy_static v1.4.0
2020-04-04T19:22:13.2269467Z    Compiling yaml-rust v0.4.3
2020-04-04T19:22:17.9746998Z    Compiling quote v1.0.2
2020-04-04T19:22:34.3620482Z    Compiling thiserror-impl v1.0.5
2020-04-04T19:22:39.5961837Z    Compiling thiserror v1.0.5
2020-04-04T19:22:39.6566342Z    Compiling yaml-merge-keys v0.4.0
2020-04-04T19:22:40.9384956Z    Compiling expand-yaml-anchors v0.1.0 (/checkout/src/tools/expand-yaml-anchors)
2020-04-04T19:22:42.7066630Z Build completed successfully in 0:00:32
2020-04-04T19:22:42.7074284Z + python2.7 ../x.py check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu
2020-04-04T19:22:42.9680247Z     Finished dev [unoptimized] target(s) in 0.19s
2020-04-04T19:22:44.1507113Z Checking rustdoc artifacts (x86_64-unknown-linux-gnu -> i686-pc-windows-gnu)
---
2020-04-04T19:24:56.3213436Z     Checking rustc_span v0.0.0 (/checkout/src/librustc_span)
2020-04-04T19:25:01.2909361Z     Checking rustc_errors v0.0.0 (/checkout/src/librustc_errors)
2020-04-04T19:25:02.6375423Z     Checking rustc_feature v0.0.0 (/checkout/src/librustc_feature)
2020-04-04T19:25:02.6563925Z     Checking fmt_macros v0.0.0 (/checkout/src/libfmt_macros)
2020-04-04T19:25:02.8689600Z     Checking rustc_query_system v0.0.0 (/checkout/src/librustc_query_system)
2020-04-04T19:25:03.6409581Z     Checking rustc_hir v0.0.0 (/checkout/src/librustc_hir)
2020-04-04T19:25:03.7610171Z     Checking rustc_session v0.0.0 (/checkout/src/librustc_session)
2020-04-04T19:25:05.4034850Z     Checking rustc_attr v0.0.0 (/checkout/src/librustc_attr)
2020-04-04T19:25:05.9243918Z     Checking rustc_parse v0.0.0 (/checkout/src/librustc_parse)
---
2020-04-04T19:29:43.4454988Z Build completed successfully in 0:00:46
2020-04-04T19:29:43.4462250Z + /scripts/validate-toolstate.sh
2020-04-04T19:29:43.4515780Z Cloning into 'rust-toolstate'...
2020-04-04T19:29:44.1441338Z Traceback (most recent call last):
2020-04-04T19:29:44.1442032Z   File "../../src/tools/publish_toolstate.py", line 305, in <module>
2020-04-04T19:29:44.1442532Z     cur_datetime
2020-04-04T19:29:44.1442974Z   File "../../src/tools/publish_toolstate.py", line 205, in update_latest
2020-04-04T19:29:44.1444301Z     maintainers = ' '.join('@'+name for name in MAINTAINERS[tool])
2020-04-04T19:29:44.1445443Z KeyError: u'rustc-dev-guide'
2020-04-04T19:29:44.1495501Z   local time: Sat Apr  4 19:29:44 UTC 2020
2020-04-04T19:29:44.1495501Z   local time: Sat Apr  4 19:29:44 UTC 2020
2020-04-04T19:29:44.3115355Z   network time: Sat, 04 Apr 2020 19:29:44 GMT
2020-04-04T19:29:45.7114234Z 
2020-04-04T19:29:45.7114234Z 
2020-04-04T19:29:45.7195840Z ##[error]Bash exited with code '1'.
2020-04-04T19:29:45.7211700Z ##[section]Finishing: Run build
2020-04-04T19:29:45.7270625Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/70665/merge to s
2020-04-04T19:29:45.7276214Z Task         : Get sources
2020-04-04T19:29:45.7276601Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-04-04T19:29:45.7276960Z Version      : 1.0.0
2020-04-04T19:29:45.7277226Z Author       : Microsoft
2020-04-04T19:29:45.7277226Z Author       : Microsoft
2020-04-04T19:29:45.7277622Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-04-04T19:29:45.7278097Z ==============================================================================
2020-04-04T19:29:46.0627973Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-04-04T19:29:46.0677135Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/70665/merge to s
2020-04-04T19:29:46.0763574Z Cleaning up task key
2020-04-04T19:29:46.0764881Z Start cleaning up orphan processes.
2020-04-04T19:29:46.2759213Z Terminate orphan process: pid (4262) (python)
2020-04-04T19:29:46.2815054Z ##[section]Finishing: Finalize Job

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 @rust-lang/infra. (Feature Requests)

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2020
Rollup of 7 pull requests

Successful merges:

 - rust-lang#70553 (move OS constants to platform crate)
 - rust-lang#70665 (Do not lose or reorder user-provided linker arguments)
 - rust-lang#70750 (Match options directly in the Fuse implementation)
 - rust-lang#70782 (Stop importing the float modules in documentation)
 - rust-lang#70798 ("cannot resolve" → "cannot satisfy")
 - rust-lang#70808 (Simplify dtor registration for HermitCore by using a list of destructors)
 - rust-lang#70824 (Remove marker comments in libstd/lib.rs macro imports)

Failed merges:

r? @ghost
@bors bors merged commit aafbe07 into rust-lang:master Apr 6, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 4, 2024
Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
ChrisDenton added a commit to ChrisDenton/rust that referenced this pull request Aug 5, 2024
…youxu

Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 5, 2024
…youxu

Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 7, 2024
…youxu

Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 7, 2024
…youxu

Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 7, 2024
…youxu

Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2024
Rollup merge of rust-lang#128647 - ChrisDenton:link-args-order, r=jieyouxu

Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants