Openjdk-tests: Remove systemtest dependency on wget and use curl on all platforms

Created on 8 Apr 2021  ·  18Comments  ·  Source: AdoptOpenJDK/openjdk-tests

👾 Description of the issue

We need to remove systemtest's dependency on wget and use curl on all platforms.

Except for system tests, all other tests have been switched to use curl, since we cannot use wget on z/OS and we do not want to support both curl and wget, we choose to use curl only (ref: https://github.com/AdoptOpenJDK/openjdk-tests/issues/1250).

Tips to solve this particular issue:

To fix this issue, we need to update top.xml Ant script and do the following:

  • Remove check-for-wget target and all its references;
  • Remove any existing condition from check-for-curl - so that it runs on all platforms;
  • Since we are using a single download tool, we may replace the target check-for-download-tool with the target check-for-curl;
  • Remove conditional property download-tool and use curl directly where download-tool was being used;
  • Remove any additional conditional properties, such as download-tool-security-options, and download-tool-srcfile-option and use the option for curl directly in the actual download command;
  • Test the changes in your branch by pointing a Grinder to it and running a system test (e.g. MathLoadTest_autosimd_0) on all platforms.

📋 Step by Step

To solve this issue and contribute a fix you should check the following step-by-step list. A more detailed documentation of the workflow can be found here.

  • [ ] Claim this issue: Comment below.
  • [ ] Fork the STF repository in github by simply clicking the 'fork' button.
  • [ ] Check out the forked repository
  • [ ] Create a feature branch for the issue. We do not have any naming definition for branches.
  • [ ] Commit your changes.
  • [ ] Start a Pull Request.
  • [ ] Done 👍 Ask in comments for a review :)
  • [ ] If the reviewers find some missing peaces or a problem, s/he will start a discussion with you and describe the next steps how the problem can be solved.
  • [ ] You did it 🎉 We will merge the fix in the master branch.
  • [ ] Thanks, thanks, thanks for being part of this project as an open source contributor ❤️

🤔❓ Questions

If you have any questions just ask us directly in this issue by adding a comment. You can join our community chat at Slack. Next to this you can find a general manual about open source contributions here.

enhancement good first issue system

Most helpful comment

I will add @SAY-droid427 to a Github team that can run a Grinder, and in this way @Mesbah-Alam - once you run the first Grinder, perhaps you can show how to rebuild it.

All 18 comments

Note, solving this will make the following issue unnecessary : https://github.com/adoptium/STF/issues/96.

If curl does not exist, curl cmd will fail with the proper error message. Personally, I do not think we need an extra check-for-curl ant target.

Hey! @smlambert @llxia @Mesbah-Alam Can I work on this issue as an Outreachy participant?

Hi @smlambert !
Could you please tell me what exactly I have to do in this issue?
I have understood that I have to fork the STF repository and make the changes there. It will be very helpful.
Thank you!

By the looks of @Mesbah-Alam 's description, the background here is that we could use wget or curl as a way to download stuff, but we only want and need 1 way.

to address this issue, you will edit https://github.com/adoptium/STF/blob/master/stf.build/include/top.xml to:

When you get to the test step a reviewer can and should help you

  • Test the changes in your branch by pointing a Grinder to it and running a system test (e.g. MathLoadTest_autosimd_0) on all platforms.

Note, this is a good 'second' issue, its a bit more involved than a typical good first issue. If you create a draft PR with some of those changes, it will be easier to give guidance if you have other questions. Thanks @SAY-droid427 !

@smlambert Thank you so much for the detailed explanation. I will try to do the changes and create a draft PR and then ask for further help!

@smlambert There are many instances of download-tool and download-tool-srcfile-option as shown in the image below. What changes should I make to them? Should I completely remove these lines?
STF

@Mesbah-Alam - can you give @SAY-droid427 some extra guidance please?

@Mesbah-Alam It would be very helpful if you could please guide me a bit on the above problem! Thanks!

There are many instances of download-tool and download-tool-srcfile-option as shown in the image below. What changes should I make to them? Should I completely remove these lines?

Hi @SAY-droid427 ,

Thanks for working on this!

Yes, since we are are no longer supporting two different download tools and only going with curl on all platforms, you may hard-code curl directly wherever ${download-tool} is referenced.

Also, note that, ${download-tool-security-options} and ${download-tool-srcfile-option}properties should also be removed. And references of${download-tool-security-options}and ${download-tool-srcfile-option} should be replaced with whatever options we have for curl.

So, in this macro, reference of ${download-tool-security-options} should be replaced with -kL. Reference of ${download-tool-srcfile-option} should be replaced by -o.

Also, as you may have noted in Lan's comment in https://github.com/AdoptOpenJDK/openjdk-tests/issues/2483#issuecomment-815903097, we no longer need to "check if the download tool" (e.g. curl in our new case) exists on the machine, which means we can remove the following targets and all their references: check-for-download-tool, check-for-wget, check-for-curl.

Please let me know if you need more help!

@Mesbah-Alam Thank you so much for the detailed explanation! I will make the necessary changes and let you know if I have more doubts!

@smlambert @Mesbah-Alam @llxia I have made a pull request with the changes. Please review it and let me know if I have missed anything else. Thank you!

Eclipse

Do I need to create an account on Eclipse?

Yes please do. Because we are in the process of moving all of our repositories into Eclipse Foundation (under the adoptium github organization), anyone making contributions to repos in that organization need to create an Eclipse account and sign the ECA agreement.

@smlambert Done

@smlambert Could anyone please help me in testing the changes by pointing a Grinder? Thanks!

@SAY-droid427 , Hi, yes, I'll take care of testing your changes.

I will add @SAY-droid427 to a Github team that can run a Grinder, and in this way @Mesbah-Alam - once you run the first Grinder, perhaps you can show how to rebuild it.

Was this page helpful?
0 / 5 - 0 ratings