Skip to content

Plugin interface cleanup - #85296

Merged
bors merged 1 commit into
rust-lang:masterfrom
bjorn3:plugin_cleanup
Aug 12, 2021
Merged

Plugin interface cleanup#85296
bors merged 1 commit into
rust-lang:masterfrom
bjorn3:plugin_cleanup

Conversation

@bjorn3

@bjorn3bjorn3 commented May 14, 2021

Copy link
Copy Markdown
Member

The first commit performs two uncontroversial cleanups. The second commit removes #[plugin_registrar] and instead requires you to export a __rustc_plugin_registrar function, this will require a change to servo's script_plugins (cc @jdm)

@bjorn3bjorn3 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-plugins Area: compiler plugins (removed in https://github.com/rust-lang/rust/pull/116412) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 14, 2021
@rust-highfive

rust-highfive commented May 14, 2021

Copy link
Copy Markdown
Contributor

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 14, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@petrochenkov

petrochenkov commented May 14, 2021

Copy link
Copy Markdown
Contributor

r? @petrochenkov

@rust-log-analyzer

This comment has been minimized.

Sign in to view
petrochenkov
petrochenkov reviewed May 15, 2021
View reviewed changes
Comment threadcompiler/rustc_feature/src/removed.rs Outdated
@petrochenkov

petrochenkov commented May 15, 2021

Copy link
Copy Markdown
Contributor

I think removing #[plugin_registrar] is fine.
The functionality is deeply deprecated and as long as servo still can access it somehow it doesn't matter much how exactly it happens, through an attribute or through an exported symbol.

@petrochenkov

petrochenkov commented May 15, 2021

Copy link
Copy Markdown
Contributor

Could you add a test making sure that two plugins from two different crates still can be loaded together despite the identical names of the plugin registrar symbols?

@petrochenkovpetrochenkov 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 May 15, 2021
@bors

This comment has been minimized.

Sign in to view
@crlf0710

crlf0710 commented Jun 5, 2021

Copy link
Copy Markdown
Member

@bjorn3 Ping from triage, any updates on this?

@JohnCSimon

JohnCSimon commented Jun 20, 2021

Copy link
Copy Markdown

@bjorn3 Ping again from triage, any updates on this?

@bjorn3

bjorn3 commented Jun 23, 2021

Copy link
Copy Markdown
MemberAuthor

Rebased, but I still need to at the requested test.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3force-pushed the plugin_cleanup branch 2 times, most recently from 447dac6 to 8611d9bCompareAugust 8, 2021 10:16
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@petrochenkov

petrochenkov commented Aug 8, 2021

Copy link
Copy Markdown
Contributor

r=me with the failing test updated and commits squashed.

@bjorn3
bjorn3force-pushed the plugin_cleanup branch 4 times, most recently from 2589a62 to 023936aCompareAugust 9, 2021 19:57
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3bjorn3 mentioned this pull request Aug 10, 2021
Merged
5 tasks
@bjorn3

bjorn3 commented Aug 10, 2021

Copy link
Copy Markdown
MemberAuthor

Opened servo/servo#28564 for the servo changes.

r=me with the failing test updated and commits squashed.

Fixed all failing tests and squashed all commits.

@bors r=petrochenkov

@bors

bors commented Aug 10, 2021

Copy link
Copy Markdown
Collaborator

📌 Commit a501308 has been approved by petrochenkov

@borsbors 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 Aug 10, 2021
@bors

bors commented Aug 12, 2021

Copy link
Copy Markdown
Collaborator

⌛ Testing commit a501308 with merge eb2226b...

@bors

bors commented Aug 12, 2021

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing eb2226b to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Aug 12, 2021
@bors
bors merged commit eb2226b into rust-lang:masterAug 12, 2021
@rustbotrustbot added this to the 1.56.0 milestone Aug 12, 2021
This was referenced Aug 12, 2021
Merged
Merged
@bjorn3
bjorn3 deleted the plugin_cleanup branch August 13, 2021 11:06
bors-servo added a commit to servo/servo that referenced this pull request Aug 13, 2021
@bors-servo
Auto merge of#28564- bjorn3:patch-1, r=jdm
ae7b978
Update script_plugin for rust-lang/rust#85296
This will update the script_plugin for the plugin interface changes in rust-lang/rust#85296.
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)
<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because it is a fix for rustc changes.
bors-servo added a commit to servo/servo that referenced this pull request Aug 14, 2021
@bors-servo
Auto merge of#28564- bjorn3:patch-1, r=jdm
3a56027
Update script_plugin for rust-lang/rust#85296
This will update the script_plugin for the plugin interface changes in rust-lang/rust#85296.
---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [ ] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)
<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because it is a fix for rustc changes.
@ehussehuss mentioned this pull request Aug 14, 2021
Merged
bors added a commit to rust-lang/cargo that referenced this pull request Aug 15, 2021
@bors
Auto merge of#9790- ehuss:fix-plugin-registrar, r=alexcrichton
ddf9adb
Fix plugin registrar change.
The plugin tests were broken due to a change in rust-lang/rust#85296 which removed the `plugin_registrar` attribute.
ehuss pushed a commit to ehuss/cargo that referenced this pull request Aug 20, 2021
@bors@ehuss
Auto merge ofrust-lang#9790- ehuss:fix-plugin-registrar, r=alexcric…
09eb666
…hton
Fix plugin registrar change.
The plugin tests were broken due to a change in rust-lang/rust#85296 which removed the `plugin_registrar` attribute.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petrochenkovpetrochenkovpetrochenkov left review comments

Assignees

@petrochenkovpetrochenkov

Labels

A-pluginsArea: compiler plugins (removed in https://github.com/rust-lang/rust/pull/116412)C-cleanupCategory: PRs that clean code up or issues documenting cleanup.merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.56.0

Development

Successfully merging this pull request may close these issues.

9 participants

@bjorn3@rust-highfive@rust-log-analyzer@petrochenkov@bors@crlf0710@JohnCSimon@matthewjasper@rustbot