ci-jenkins-pipelines
ci-jenkins-pipelines copied to clipboard
Now buildign also jcstress-tests-all-20240222.jar 20220908.jar
Those are based on closest (which were actually that day last) commit hashes. junit tests are passing.
In addition shortened hash to 7 latters and uses jdk17 to tip
Should be fixing https://github.com/adoptium/ci-jenkins-pipelines/issues/1016
Thank you for creating a pull request!
Please check out the information below if you have not made a pull request here before (or if you need a reminder how things work).
Code Quality and Contributing Guidelines
If you have not done so already, please familiarise yourself with our Contributing Guidelines and Code Of Conduct, even if you have contributed before.
Tests
Github actions will run a set of jobs against your PR that will lint and unit test your changes. Keep an eye out for the results from these on the latest commit you submitted. For more information, please see our testing documentation.
In order to run the advanced pipeline tests (executing a set of mock pipelines), it requires an admin to post run tests
on this PR.
If you are not an admin, please ask for one's attention in #infrastructure on Slack or ping one here.
To run full set of tests, use "run tests"; a subset of tests on specific jdk version, use "run tests quick 11,21"
We do not need a build from that date. We need/want to use tagged or released builds. In order to do that, we need to 'test' that an appropriately recent tagged build can run the test targets we have, without major errors.
So indeed, please do not bother building untagged/untracked versions of jcstress. I do not have time to review this PR today as I am on 'personal time off'. Please do not rush into merging anything. There is all the time in the world to get things right and yet takes no time at all to do the wrong thing.
This PR is not in rush for sure. Whatever we agree should be built, will be built.
However I think tagged builds are not an option. jcstress team (read shade) tags somehow randomly, and last tag is pretty old. Also every time I reported an issue, first order was update jcstress, and if not to some particular commit,then to head. And indeed it worked in 3/4 of cases. Similarly the https://mvnrepository.com/artifact/org.openjdk.jcstress is doing that. IIRC, I was trying to do maven releases more often, and I still have permissions to do that, but that is still jsut random local build. Same counts for JMH. And you still need one of project owners to tag the repo, and that may be show-stopper pretty often.
Also I doubt we wish to go with tip, as that can indeed change dramatically without warning. From that point of view the 20240222 is pretty good option as it is known to be stable and is really fresh.
I was following https://github.com/adoptium/aqa-tests/issues/5261 where you gave me green light, unluckily only during voice chat, and it literally says:
improve https://ci.adoptium.net/view/Dependencies/job/dependency_pipeline/lastSuccessfulBuild/artifact/jcstress/ so it builds jcstress-tests-all-20220908.jar and jcstress-tests-all-20240222.jar and tip
Which is what this PR is doing (tip was already there).
If you reconsidered, it is ok and I will follow for sure, I do not have intentions to break anything, but I would like to have aqa-tests jcstress build on better foundation then on random jcstress-tests-all-20220908.jar which was downloaded ages ago and can no longer be recreated. (Or I have not found it). So indeed the builder should be building 20220908 - so the https://github.com/adoptium/aqa-tests/blob/master/system/jcstress/playlist.xml and https://github.com/adoptium/TKG/blob/master/scripts/getDependencies.pl#L182 can immediately reuse that. Then it should be able to pick up 20240222 moreover out of the box. Maybe except jdk8 and maybe only on demand.. that will follow.
Generally it is good we had this conversation. I realised, that all above is valid also for JMH, and that jcstres palylist and/or https://github.com/adoptium/TKG/blob/master/scripts/getDependencies.xml have to be able to work - on demand only! - with jcstress tip, as that is waht jcstress people will always ask us to do first.
No rush from me. Do not spoil your time off. You made yourself clear you wish to double check that, and I'm greatful for it.
Looking to https://github.com/adoptium/aqa-tests/issues/5278 (and seeing https://github.com/adoptium/aqa-tests/pull/5270 work ok) This shold be building only 20240222, and lets say with "correct" name of jcstress-tests-all-20240222.jar
?
Currently aqa-tests is grabing jcstress from personal server so this PR won't affect aqa-tests. I think 20240222 is enough ( no need to build 20220908.jar).
What's the difference of building the jar with jdk17 and jdk11? Do we need both version? Once this is in we can update aqa-tests.
Currently aqa-tests is grabing jcstress from personal server so this PR won't affect aqa-tests. I think 20240222 is enough ( no need to build 20220908.jar).
Right and right. I will drop the 2022 version as the 2024 is already "iun production" (was not when I strarted thi sPR) But immediately once it is in I will switch the wget of it as descibed in https://github.com/adoptium/aqa-tests/issues/5261 from personal server, to result of this build (latestSucesfullBuild jcstress/Artifacts)
What's the difference of building the jar with jdk17 and jdk11? Do we need both version? Once this is in we can update aqa-tests.
That appeared to be wrong assumption. I will swithc it all to jdk8.At least we will know, when jdk8 is not worthy anymore.
Please double check the naming of main tarball.
I'm using: jcstress.jar, jcstress-0.16.jar jcstress-20240222.jar, jcstress-somehash.jar and jcstress-tip.jar, (last two and first two are hardlink). The someHash one is now jcstress-d6a81c3.jar
Where this is more aligned to how maven is generating the jars, current usage in adoptium is jcstress-tests-all-20240222.jar
which is slightly more describing waht directroy the tests are from. However I think it is not necessary to highlight because it is the only jar produced during default build. Note, that the old naming is used in both current generators (see https://github.com/adoptium/aqa-tests/issues/5278). But that woudl be super easy to adjust onc ethis is in. TY!
That appeared to be wrong assumption. I will swithc it all to jdk8.At least we will know, when jdk8 is not worthy anymore.
Not sure what do you mean switch it all to jdk8. The project mentioned The project requires JDK 11+ to build.
https://github.com/openjdk/jcstress ?
Sorry, I overwrote. See the PR, I switched to 11. I ment 11. My wrong for that "typo"/
https://ci.adoptium.net/view/Dependencies/job/dependency_pipeline/1186/artifact/jcstress/
Are jcstress-0.16.jar, jcstress-20240222.jar, jcstress-d6a81c3.jar different? If not we don't need to keep all of them?
Could you @judovana make the PR ready for review if it's ready?
The linking follows all other ci-pipellines buildjobs.
- tip have hash in name, and is linked as tip (jcstress-d6a81c3.jar +jcstress-tip.jar)
- latest release have release in name and is linked as "that" (jcstress-0.16.jar + jcstress.jar)
- jcstress-20240222.jar as artificial jar for internal purposes have no linking
The goal is to have predictable names for pernament links, and still haveing clear what is waht.
The jcstress-20240222.jar is now very similar to jcstress-d6a81c3.jar because only few commits was added in meantime, but it is ill idea to have internal testing agasint tip.
So jcstress.jar, jcstress-d6a81c3.jar and jcstress-tip.jar are same? For now I think tip, latest release, jcstress-20240222.jar are enough as long as the sha is provided.
So jcstress.jar, jcstress-d6a81c3.jar and jcstress-tip.jar are same? For now I think tip, latest release, jcstress-20240222.jar are enough as long as the sha is provided.
no jcstress-d6a81c3.jar and jcstress-tip.jar are same, and then jcstress.jar anf (jcstress-0.16.jar as I wrote above.
All other ci-tools are keeping this convention. So I would keep it.
tyvm! for approve!
@karianna any suggestion?
@karianna Hello, can you reevaluate?
Yes, in absence of a stable release tag to build off of, we proceed with this approach. If jcstress begins to employ release tagging in future, we can adjust.