Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

clarify and document needs-dynamic-linking #135336

Merged
merged 3 commits into from
Feb 12, 2025
Merged

clarify and document needs-dynamic-linking #135336

merged 3 commits into from
Feb 12, 2025

Conversation

tshepang
Copy link
Member

@tshepang tshepang commented Jan 10, 2025 •
edited
Loading

try-job: test-various

@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2025

r? @cjgillot

rustbot has assigned @cjgillot.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 10, 2025

The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes.

cc @BoxyUwU, @jieyouxu, @Kobzol

@rustbot rustbot added the A-rustc-dev-guide Area: rustc-dev-guide label Jan 10, 2025
jieyouxu
jieyouxu approved these changes Jan 10, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks!

@jieyouxu jieyouxu assigned jieyouxu and unassigned cjgillot Jan 10, 2025
@jieyouxu
Copy link
Member

@bors r+ rollup=iffy

@bors
Copy link
Contributor

bors commented Jan 10, 2025

📌 Commit 3e9cbc1 has been approved by jieyouxu

It is now in the queue for this repository.

@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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 10, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 11, 2025
@bors
Auto merge of rust-lang#135336 - tshepang:patch-5, r=jieyouxu
f8ea063 
use target feature and not target name

Also, modify 1 test to test cli functionality in all targets
@bors
Copy link
Contributor

bors commented Jan 11, 2025

⌛ Testing commit 3e9cbc1 with merge f8ea063...

@rust-log-analyzer

This comment has been minimized.

# to view
@bors
Copy link
Contributor

bors commented Jan 11, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 11, 2025
@jieyouxu jieyouxu mentioned this pull request Jan 11, 2025
Open
@jieyouxu
Copy link
Member

@bors retry (#134351)

@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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 11, 2025
@bors
Copy link
Contributor

bors commented Jan 12, 2025

⌛ Testing commit 3e9cbc1 with merge 8e4ea5e...

bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 12, 2025
@bors
Auto merge of rust-lang#135336 - tshepang:patch-5, r=jieyouxu
8e4ea5e 
use target feature and not target name

Also, modify 1 test to test cli functionality in all targets
@rust-log-analyzer

This comment has been minimized.

# to view
@bors
Copy link
Contributor

bors commented Jan 12, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 12, 2025
@jieyouxu
Copy link
Member

jieyouxu commented Jan 12, 2025 •
edited
Loading

Right... I seem to recall supporting dynamic_linking != supporting dylib. This is #132309.

@jieyouxu jieyouxu 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 Jan 12, 2025
@tshepang
Copy link
Member Author

tshepang commented Feb 11, 2025 •
edited
Loading

@jieyouxu should I submit this chunk as a separate pr

what about 3e9cbc1... should it also be a separate pr

jieyouxu
jieyouxu requested changes Feb 11, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry I dug up some past issues I somehow forgor, the r-d-g changes certainly make sense to be in this PR as well (this is like the whole reason why we really wanted to make r-d-g a subtree in this repo in the first place).

Let's just keep these changes in this PR, they make sense together.

@tshepang tshepang force-pushed the patch-5 branch 2 times, most recently from ae98400 to 7474e2b Compare February 11, 2025 20:03
@tshepang
Copy link
Member Author

@jieyouxu ready now

jieyouxu
jieyouxu approved these changes Feb 11, 2025
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, just one wording nit. You can r=me after the wording nit.

@tshepang
test cli functionality in all targets
3e50873
@tshepang
clarify
d5f6456 
Also, use signular form for consistency/simplicity
@tshepang
Copy link
Member Author

@bors r=jieyouxu

@bors
Copy link
Contributor

bors commented Feb 12, 2025

@tshepang: 🔑 Insufficient privileges: Not in reviewers

@tshepang tshepang changed the title use target feature and not target name clarify and document needs-dynamic-linking Feb 12, 2025
@jieyouxu
Copy link
Member

Thanks!
@bors r+

@bors
Copy link
Contributor

bors commented Feb 12, 2025

📌 Commit d5f6456 has been approved by jieyouxu

It is now in the queue for this repository.

@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 Feb 12, 2025
@bors
Copy link
Contributor

bors commented Feb 12, 2025

⌛ Testing commit d5f6456 with merge ced8e65...

@bors
Copy link
Contributor

bors commented Feb 12, 2025

☀️ Test successful - checks-actions
Approved by: jieyouxu
Pushing ced8e65 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 12, 2025
@bors bors merged commit ced8e65 into rust-lang:master Feb 12, 2025
7 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 12, 2025
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (ced8e65): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.3%, secondary -4.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -4.6% [-4.6%, -4.6%] 1
All ❌✅ (primary) 2.3% [2.3%, 2.3%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 789.011s -> 788.874s (-0.02%)
Artifact size: 347.78 MiB -> 347.76 MiB (-0.00%)

@tshepang tshepang deleted the patch-5 branch February 14, 2025 10:08
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels
A-rustc-dev-guide Area: rustc-dev-guide merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.86.0
Development

Successfully merging this pull request may close these issues.
7 participants
@tshepang @rustbot @jieyouxu @bors @rust-log-analyzer @rust-timer @cjgillot