hibernate-validator icon indicating copy to clipboard operation
hibernate-validator copied to clipboard

Update dependabot config and update dependencies where possible

Open marko-bekhta opened this issue 1 year ago β€’ 7 comments

Hey @gsmet πŸ˜ƒ

after opening https://github.com/hibernate/hibernate-validator/pull/1344 because of the issues building the project when using the JDK 21 and doing the SM removal work in https://github.com/hibernate/hibernate-validator/pull/1345 I thought that maybe we should enable dependabot and keep the build of the Validator more up to date so it'll be more straightforward to address any build issues rather than waiting for them to pileup...

I've used the similar dependabot config that we have in the Search. The only thing I couldn't upgrade is Wildfly because of https://github.com/jbossas/patch-gen/issues/34 πŸ˜” but it looks like that project is a bit stale πŸ˜”πŸ˜”πŸ˜”

marko-bekhta avatar Apr 18 '24 09:04 marko-bekhta

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HV-\d+     ↳ Offending commits: [3c225c3bc576ff2f9adb709843e313be1ac2ba83, 42d15df90e2955453ae2a7101f8706b5d475f162]

β€Ί This message was automatically generated.

I would say +1 but CI looks unhappy?

gsmet avatar Apr 18 '24 11:04 gsmet

DEBUG: figure out why TCK test failing on CI only

Aw shoot, I'm missing all the fun :grin:

yrodiere avatar Apr 19 '24 08:04 yrodiere

DEBUG: figure out why TCK test failing on CI only

Aw shoot, I'm missing all the fun 😁

πŸ™ˆ πŸ˜„ mmmmmmmm ooooook... so the problem is caused by:

file:/var/lib/jenkins/workspace/hibernate-validator_PR-1347%40tmp/maven-repository/org/assertj/assertj-core/3.8.0/assertj-core-3.8.0.jar
file.exists() = false
file.isFile() = false
file.listFiles() = null

Which, if we don't upgrade the surefire plugin, is:

file:/var/lib/jenkins/workspace/hibernate-validator_PR-1347@tmp/maven-repository/org/assertj/assertj-core/3.8.0/assertj-core-3.8.0.jar
file.exists() = true
file.isFile() = true
file.listFiles() = null

For some reason with the plugin upgrade @ gets encoded in the path, and I'm not sure how that's connected since the URL comes from:

Assert.class.getProtectionDomain().getCodeSource().getLocation()

which isn't surefire related πŸ€”πŸ˜•

and that's happening inside of the TCK classes .... it's fixable, but I guess we aren't getting a new version of TCK to make this work πŸ€”

marko-bekhta avatar Apr 19 '24 10:04 marko-bekhta

For some reason with the plugin upgrade @ gets encoded in the path

I remember similar problems from when we worked on running Jandex indexing on startup in Hibernate Search... I don't remember what I changed to fix it, but here's the test that checked it worked, and I suppose the fix is somewhere in the same commit: https://github.com/hibernate/hibernate-search/commit/2773f906cdccfacabec67ea02c02f16e0a197efb#diff-fa81f1bd295eb29f597f67849a355691e3672277da2aa60d41156433b3935971R257-R258

Does that help in any way?

yrodiere avatar Apr 22 '24 08:04 yrodiere

Thanks πŸ˜ƒ I've opened https://github.com/jakartaee/validation-tck/pull/198, since the access to the code source happens inside the TCK classes, calling .toURI() helps, at least in the tests I've run locally. But since it's all TCK code, I guess it's one of:

  • we get a new TCK
  • we don't upgrade the surefire plugin (that, for some reason, causes the issue)
  • we do something to Jenkins jobs that they do not include any chars that get encoded in the file paths

marko-bekhta avatar Apr 22 '24 08:04 marko-bekhta

I've opened jakartaee/validation-tck#198, since the access to the code source happens inside the TCK classes, calling .toURI() helps

Thanks. @gsmet could you add your +1 to that PR, since I guess your opinion has great weight over there? I imagine Scott is quite busy right now, so your advice could help expedite things.

we get a new TCK

Probably a good idea if we merge this in HV 9.0 with Jakarta 11 support.

we do something to Jenkins jobs that they do not include any chars that get encoded in the file paths

That's going to be challenging, as the part of the path that causes the problem is somewhat standard (it's a temp dir that's available to each job) and not configurable AFAICS.

we don't upgrade the surefire plugin (that, for some reason, causes the issue)

Or... we upgrade, but find some config option that reverts to the old behavior. Not sure if this is possible though.

yrodiere avatar Apr 22 '24 09:04 yrodiere

https://ci.hibernate.org/blue/organizations/jenkins/hibernate-validator/detail/PR-1346/25/pipeline

The build is passing now ^. But with all the changes, this branch essentially became 9.0

marko-bekhta avatar Jun 12 '24 14:06 marko-bekhta

https://ci.hibernate.org/blue/organizations/jenkins/hibernate-validator/detail/PR-1346/25/pipeline

The build is passing now ^. But with all the changes, this branch essentially became 9.0

Okay then, I guess we should branch out 8.0 and make main the 9.0.

Can you please take care of that?

What do we need to merge to 8.0 and possibly earlier branches? From what I can see these are the only potential candidates:

  • https://hibernate.atlassian.net/browse/HV-1981 Use Maven 3.9.6 in CI builds and as a required minimum version for the build -- seems to make sense for 8.0, not sure about other branches
  • https://hibernate.atlassian.net/browse/HV-1980 Update gpg plugin configuration -- probably needed on all branches that actually have gpg configuration
  • https://hibernate.atlassian.net/browse/HV-1992 Switch to a different sigtest-maven-plugin -- perhaps not necessary on JDK 11?

yrodiere avatar Jun 12 '24 15:06 yrodiere

We have 6.2 and 8.0, so I'm thinking that since 6.2 is javax, we probably can just add the gpg patch to it. As for the sigtest plugin it only affected the branch with the bumped JDK, so we can leave it for 9.0.

marko-bekhta avatar Jun 12 '24 16:06 marko-bekhta

Can you please take care of that?

OK πŸ˜ƒ

  • I've applied patches to 6.2 and main
  • created 8.0 (including the patches)
  • added one more commit to this branch (the first one) to bump the version to 9.0

marko-bekhta avatar Jun 12 '24 17:06 marko-bekhta