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

Switch BOM pom.xml to v.r.* ranges for included specs to allow for security fixes etc. #132

Closed
wants to merge 3 commits into from

Conversation

hutchig
Copy link
Member

@hutchig hutchig commented Oct 14, 2019

Signed-off-by: Gordon Hutchison [email protected]

As requested in the Oct 8th Architecture call minutes
This PR switches the BOM to allow ranges for the third digit (second decimal) version numbers
for included specs.

This is to allow for issues such as CIE alerts in transitive dependencies to be handled better.

So the POM still specifies particular X.Y ranges but will pick up the latest Z in X.Y.Z
(a change to X may be a breaking change, a change to Y alone
should be backwards compatible with existing users of any API).
So for X.Y.Z a variation in Z alone should have identical API but allow for
security and maintenance fixes from individual specs to be picked up.

Both Jakarta and Microprofile use this convention.

(I checked metrics version going from 3 to 2 digits (2.1.0 to 2.1.*) has no effect)

See here, for example for the Maven syntax used.

@hutchig hutchig changed the title Switched BOM pom.xml to v.r.* ranges for included specs to allow for security fixes etc. Switch BOM pom.xml to v.r.* ranges for included specs to allow for security fixes etc. Oct 14, 2019
pom.xml Outdated Show resolved Hide resolved
Copy link

@kenfinnigan kenfinnigan left a comment

Choose a reason for hiding this comment

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

Looks good.

Should probably have @kwsutter or @jclingan approve this as well

Copy link
Member

@xstefank xstefank left a comment

Choose a reason for hiding this comment

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

+1

@hutchig
Copy link
Member Author

hutchig commented Oct 16, 2019

See #134 (comment) for some interesting points from @joakime

@hutchig hutchig added the HOLD label Oct 16, 2019
@hutchig
Copy link
Member Author

hutchig commented Oct 16, 2019

HOLD added so we can digest #132 (comment)

@hutchig
Copy link
Member Author

hutchig commented Oct 18, 2019

Please could you review this @Emily-Jiang ?

@hutchig hutchig removed the HOLD label Oct 18, 2019
@hutchig
Copy link
Member Author

hutchig commented Oct 18, 2019

I have removed the HOLD label as I have thoroughly investigated the exact
maven range lower bounds and added the .alpha decorator to the non inclusive
upper bounds of the ranges as by definition that is the very very lowest version
of the 'next' version up.

@Azquelt
Copy link
Member

Azquelt commented Oct 18, 2019

I think the ranges as they are now would still pick up a micro version release candidate (e.g. 1.0.1-RC1). Not sure if that's an issue or not.

@hutchig
Copy link
Member Author

hutchig commented Oct 18, 2019

Good spot @Azquelt, I believe the way round that would be to use a rule to ignore such:

<plugin>
    <groupId>org.codehaus.mojo</groupId>
    <artifactId>versions-maven-plugin</artifactId>
    <version>2.1</version>
    <configuration>
        <rulesUri>file:///${session.executionRootDirectory}/rules.xml</rulesUri>
    </configuration>
</plugin>
.... where rules.xml is 
<ruleset 
  comparisonMethod="maven"
  xmlns="http://mojo.codehaus.org/versions-maven-plugin/rule/2.0.0"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://mojo.codehaus.org/versions-maven-plugin/rule/2.0.0 http://www.mojohaus.org/versions-maven-plugin/xsd/rule-2.0.0.xsd">

  <rule groupId="org.eclipse.microprofile">
    <ignoreVersions>
      <ignoreVersion type="regex">.*[-_\.](alpha|Alpha|ALPHA|b|beta|Beta|BETA|rc|‌​RC|M|EA)[-_\.]?[0-9]*</ignoreVersion>
    </ignoreVersions>
  </rule>
</ruleset>

@hutchig hutchig added the HOLD label Oct 18, 2019
@hutchig
Copy link
Member Author

hutchig commented Oct 18, 2019

I put the HOLD back on as I want to investigate how this rule works on transitive dependencies (i.e distributing the rules so they are active later. This is possible I believe: https://stackoverflow.com/questions/26120704/maven-versions-plugin-reference-a-rule-xml-from-a-maven-dependency

@kwsutter
Copy link
Contributor

I read the minutes from the architecture call, and I'm still confused as to what we are trying to accomplish. This pom file is really only used when releasing a new version of the MicroProfile platform. If a component has a security patch (or whatever), then they should just submit a PR against the pom.xml to get the upper bounds updated for their component. I don't think we should be automatically picking up third digit updates without some indication from the component that they should be picked up for the platform. An explicit PR would accomplish that.

@hutchig
Copy link
Member Author

hutchig commented Oct 18, 2019

I am fine with what Kevin said. It was an interesting PR for me to do but I don't think this is the
time or place to do a custom ruleset etc. So if no one objects by Tuesday's call I will close
this PR then.

@hutchig
Copy link
Member Author

hutchig commented Oct 22, 2019

Closing as per #133

@hutchig hutchig closed this Oct 22, 2019
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.

5 participants