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

Update README and add Java 8, 11, 16 Temurin Debian, Red Hat and Suse #332

Merged
merged 39 commits into from
Sep 8, 2021

Conversation

karianna
Copy link
Contributor

@karianna karianna commented Aug 6, 2021

  • Update gitignore
  • Update README.md to reflect how scripts work today
  • First cut of Java 8 Temurin Debian files

@karianna karianna marked this pull request as draft August 6, 2021 14:45
@karianna karianna mentioned this pull request Aug 6, 2021
13 tasks
@karianna karianna changed the title Update README and add Java 8 Temruin Debian as as a draft Update README and add Java 8 Temurin Debian and Red Hat as as a draft Aug 6, 2021
@karianna
Copy link
Contributor Author

karianna commented Aug 6, 2021

@aahlenst - Would it be OK for you to review this PR and see if it makes sense to you? It's still a WIP but I want to make sure I'm heading in roughly the right direction here :-)

@karianna
Copy link
Contributor Author

karianna commented Aug 6, 2021

I need to get an RPM-based VM now don't I. Anything you'd recommend for me to spin up simply in VirtualBox (yeah, yeah I should be Docker pro here but I'm a dinosaur and still like my VMs)

@aahlenst
Copy link
Contributor

aahlenst commented Aug 6, 2021

@karianna Will do, give me a couple of days.

@aahlenst
Copy link
Contributor

aahlenst commented Aug 6, 2021

The list of tools included in the JDK can for example be pulled from https://github.com/adoptium/installer/blob/master/linux/deb/config/jdk-8-hotspot/tools.txt. Might need an update if OpenJDK added or dropped tools lately. Lack of pack200 stood out, for example.

Copy link
Contributor

@aahlenst aahlenst left a comment

Choose a reason for hiding this comment

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

I suppose the spec file for Suse comes at a later stage?

@aahlenst
Copy link
Contributor

aahlenst commented Aug 7, 2021

I need to get an RPM-based VM now don't I. Anything you'd recommend for me to spin up simply in VirtualBox (yeah, yeah I should be Docker pro here but I'm a dinosaur and still like my VMs)

For final verification of the package, why not. But you can run the build incl. tests on any computer running Linux or even on a Mac as long as you have both Docker and a JDK. That already catches a ton of errors.

@karianna
Copy link
Contributor Author

karianna commented Aug 9, 2021

I suppose the spec file for Suse comes at a later stage?

Yes, once I'm confident with Debian and RH :-)

@karianna
Copy link
Contributor Author

karianna commented Aug 9, 2021

@aahlenst Thanks for the detailed review, I've updated everything except for the RPM spec file (I need to still update the tools there).

@karianna karianna changed the title Update README and add Java 8 Temurin Debian and Red Hat as as a draft Update README and add Java 8, 11, 16 Temurin Debian, Red Hat and Suse Aug 10, 2021
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

I haven't reviewed all of it yet, but here are a small number of initial comments

linuxNew/README.md Outdated Show resolved Hide resolved
linuxNew/README.md Outdated Show resolved Hide resolved
linuxNew/README.md Outdated Show resolved Hide resolved
linuxNew/README.md Outdated Show resolved Hide resolved
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

One suggestion to make a clarification in a comment in the code based on the responses to my feedback, but otherwise seems good.

@sxa sxa self-requested a review August 12, 2021 16:53
@gdams
Copy link
Member

gdams commented Aug 25, 2021

Test failures CC @karianna @aahlenst

@karianna
Copy link
Contributor Author

Yeah, I've posted in the issue tracking this. It's a file size difference and I'm really not sure if its a test data issue here.

linuxNew/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jerboaa jerboaa left a comment

Choose a reason for hiding this comment

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

This seems fine for an initial cut.

karianna and others added 3 commits September 8, 2021 11:12
@karianna karianna marked this pull request as ready for review September 8, 2021 10:16
@karianna karianna requested a review from aahlenst September 8, 2021 10:16
@karianna
Copy link
Contributor Author

karianna commented Sep 8, 2021

@aahlenst I can't figure out why the ZypperTests fail intermittently. Running those checks locally always passes for me. https://github.com/adoptium/installer/runs/3543407221?check_suite_focus=true is an example of the failure

@aahlenst
Copy link
Contributor

aahlenst commented Sep 8, 2021

For easier analysis of test failures in CI it's best to collect Gradle's reports. Something like:

- uses: actions/upload-artifact@v2
  if: failure()
  with:
    name: gradle-reports
    path: |
      linuxNew/**/build/reports

Gradle captures stdout and stderr, so we should see the output of Zypper and other tools.

@karianna
Copy link
Contributor Author

karianna commented Sep 8, 2021

Looks like potentially fixing up the hkps access for keys has resolved the last of the issues.

@karianna karianna merged commit 163e753 into adoptium:master Sep 8, 2021
@karianna karianna deleted the update_linuxNew branch September 16, 2021 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants