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

Allow UCL sign option for Windows installer #321

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AdamBrousseau
Copy link
Contributor

Related adoptium/temurin-build#2525

Signed-off-by: Adam Brousseau adam.brousseau88@gmail.com

Related adoptium/temurin-build#2525

Signed-off-by: Adam Brousseau <adam.brousseau88@gmail.com>
@AdamBrousseau
Copy link
Contributor Author

Have tested this for the new code path on my farm. Added param to create_windows_installer job for SIGN_TOOL. Reccomend Adopt tests as well.

@karianna karianna added this to To do in installer via automation Jun 29, 2021
@karianna karianna added this to the June 2021 milestone Jun 29, 2021
@karianna karianna moved this from To do to In progress in installer Jun 29, 2021
@ECHO OFF
"%ProgramFiles(x86)%\Windows Kits\%WIN_SDK_MAJOR_VERSION%\bin\%WIN_SDK_FULL_VERSION%\x64\signtool.exe" sign -f "%SIGNING_CERTIFICATE%" -p "%SIGN_PASSWORD%" -fd sha256 -d "Adoptium" -t %%s "ReleaseDir\!OUTPUT_BASE_FILENAME!.msi"
@ECHO ON
IF "%SIGN_TOOL%" == "ucl" (
Copy link
Contributor

Choose a reason for hiding this comment

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

Whitespace seems off?

ucl sign-code --file "ReleaseDir\!OUTPUT_BASE_FILENAME!.msi" -n WindowsSHA -t %%s --hash SHA256
) ELSE (
@ECHO OFF
"%ProgramFiles(x86)%\Windows Kits\%WIN_SDK_MAJOR_VERSION%\bin\%WIN_SDK_FULL_VERSION%\x64\signtool.exe" sign -f "%SIGNING_CERTIFICATE%" -p "%SIGN_PASSWORD%" -fd sha256 -d "AdoptOpenJDK" -t %%s "ReleaseDir\!OUTPUT_BASE_FILENAME!.msi"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be Adoptium not AdoptOpenJDK?

@douph1
Copy link
Contributor

douph1 commented Jun 29, 2021

What is the meaning of adding dedicated code that is not used by temurin build farm ?
If we want something extensible maybe another approach without limiting to UCL will be better

something like :

if exists sign.cmd (
call sign.cmd
) else (
signtool.exe...
)

and sign.cmd can reuse all the parent var he want to sign

@karianna
Copy link
Contributor

What is the meaning of adding dedicated code that is not used by temurin build farm ?
If we want something extensible maybe another approach without limiting to UCL will be better

something like :

if exists sign.cmd (
call sign.cmd
) else (
signtool.exe...
)

and sign.cmd can reuse all the parent var he want to sign

As a philosophy, we're happy to accept changes that help other folks who use our build scripts (as long as they are not detrimental to the temurin builds). Using extensibility is a good idea here :-)

@gdams
Copy link
Member

gdams commented Oct 20, 2022

@AdamBrousseau is this still required?

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

A block has been put on this Pull Request as this repository is temporarily under a code freeze due to an ongoing release cycle.

If this pull request needs to be merged during the release cycle then please comment /merge and a PMC member will be able to remove the block.

If the code freeze is over you can remove this block by commenting /thaw.

@AdamBrousseau
Copy link
Contributor Author

Well we have the changes on our fork. Ideally we upstream as many of our changes as possible. Prevents merge conflicts and contributes our work back to the community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
installer
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

5 participants