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

Also load local restrictions from old index location #54

Merged
merged 2 commits into from
Jul 30, 2021

Conversation

jglick
Copy link
Member

@jglick jglick commented Jul 30, 2021

I had thought I could skip the local index in #53, on the grounds that

  • The checker would only be run with the new indexer in the current POM.
  • Skipping some indices would at worst cause some incorrect code to pass.

Both assumptions were apparently wrong:

  • [JENKINS-66247] Do not override the version of annotation-indexer from core plugin-pom#426 means that if ${jenkins.version} predates 2.289.x, we will indeed run a new access checker tool while the current plugin (target/classes/) has been built against an old annotation indexer.
  • Apparently something about how @Restricted(NoExternalUse.class) is implemented means that if you do not load target/classes/META-INF/annotations/org.kohsuke.accmod.Restricted, the build will fail when another class in the current source root refers to the annotated API.

@timja
Copy link
Member

timja commented Jul 30, 2021

repositories are missing in the pom i assume

@jglick
Copy link
Member Author

jglick commented Jul 30, 2021

Looks that way. Will check in #55.

@jglick jglick added the bug label Jul 30, 2021
Copy link

@MarkEWaite MarkEWaite left a comment

Choose a reason for hiding this comment

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

I'm unqualified to assess the change but am confident that @jglick is the right person to make the change. If there is a surprise, we'll adapt to the surprise.

@jglick
Copy link
Member Author

jglick commented Jul 30, 2021

Makes the IT in jenkinsci/plugin-pom#428 pass at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants