Skip to content

enabled use of InstallLocation for Oracle.JDK.17#40984

Merged
1 commit merged intomicrosoft:masterfrom
Cube707:Oracle.JDK.17
Jan 15, 2022
Merged

enabled use of InstallLocation for Oracle.JDK.17#40984
1 commit merged intomicrosoft:masterfrom
Cube707:Oracle.JDK.17

Conversation

@Cube707
Copy link
Copy Markdown
Contributor

@Cube707 Cube707 commented Jan 15, 2022

  • Have you signed the Contributor License Agreement?
  • Have you checked that there aren't other open pull requests for the same manifest update/change?
  • Have you validated your manifest locally with winget validate --manifest <path>?
  • Have you tested your manifest locally with winget install --manifest <path>?
  • Does your manifest conform to the 1.0 schema?

Note: <path> is the name of the directory containing the manifest you're submitting.


made the -l,--location option functional for this package

Microsoft Reviewers: Open in CodeFlow

@wingetbot
Copy link
Copy Markdown
Collaborator

Service Badge  Service Badge  

@wingetbot
Copy link
Copy Markdown
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Copy Markdown
Contributor

@ItzLevvie ItzLevvie left a comment

Choose a reason for hiding this comment

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

@ghost ghost added the Needs-Author-Feedback This needs a response from the author. label Jan 15, 2022
@Cube707
Copy link
Copy Markdown
Contributor Author

Cube707 commented Jan 15, 2022

hm, ok than I must have bodged up somewhere else, because the -l option wasn't working for me. I will test again....

@ghost ghost added Needs-Attention This work item needs to be reviewed by a member of the core team. and removed Needs-Author-Feedback This needs a response from the author. labels Jan 15, 2022
@Cube707
Copy link
Copy Markdown
Contributor Author

Cube707 commented Jan 15, 2022

@lutzroeder did some more Testing and even on a fresh Win10 VM it isn't working as expected:

All of the following result in a installation to the default directory, NOT to the given directory:

winget install Oracle.JDK.17 -l C:\ProgStuff\jdk
winget install Oracle.JDK.17 -l "C:\ProgStuff\jdk"
winget install Oracle.JDK.17 --override "/passive TARGETDIR=C:\ProgStuff\jdk"

The only thing that works and results in a different installation directory is:

winget install Oracle.JDK.17 --override "/passive INSTALLDIR=C:\ProgStuff\jdk"

@ItzLevvie
Copy link
Copy Markdown
Contributor

ItzLevvie commented Jan 15, 2022

Looks like you were right.

I double-checked on a VM and it looks like this installer doesn't support TARGETDIR; only INSTALLDIR.

@ghost ghost added the Moderator-Approved One of the Moderators has reviewed and approved this PR label Jan 15, 2022
@Cube707
Copy link
Copy Markdown
Contributor Author

Cube707 commented Jan 15, 2022

reading the link you provided, as I understand it, this is also the correct behavior:

INSTALLDIR represents [..] such as the end user launching Setup.exe [..]

TARGETDIR represents [..] when the user runs Setup.exe or MsiExec.exe with the /a command-line switch

the targetdir option may not be the correct option here? The /a switch for the msi process is not used or is it?

@wingetbot wingetbot added Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Validation-Completed Validation passed labels Jan 15, 2022
@ghost
Copy link
Copy Markdown

ghost commented Jan 15, 2022

Hello @wingetbot!

Because this pull request has the Validation-Completed label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost
Copy link
Copy Markdown

ghost commented Jan 15, 2022

Hello @Cube707,
Validation has completed.

@ghost ghost merged commit df50494 into microsoft:master Jan 15, 2022
@Cube707
Copy link
Copy Markdown
Contributor Author

Cube707 commented Jan 15, 2022

@ItzLevvie I tested a few more packages and there seems to be some more inconsistency.

ChristianHohnstadt.xca and StrawberryPerl.StrawberryPerl seem to suffer from the same problem while Borvid.HttpMasterExpress works absoloutely fine as is and doesn't work with the INSTALLDIR option.

just using both options at the same time seems to work fine though. At least in my quick tests.

should I open a issue about this?

@Cube707 Cube707 deleted the Oracle.JDK.17 branch January 15, 2022 15:37
@wingetbot
Copy link
Copy Markdown
Collaborator

Publish pipeline succeeded for this Pull Request. Once you refresh your index, this change should be present.

@OfficialEsco
Copy link
Copy Markdown
Contributor

OfficialEsco commented Jan 15, 2022

should I open a issue about this?

Please do, this will break waaaaaaaaaaaaay too many Packages

Just for safety, make a issue here https://github.com/microsoft/winget-cli/issues not on winget-pkgs =))

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Moderator-Approved One of the Moderators has reviewed and approved this PR Needs-Attention This work item needs to be reviewed by a member of the core team. Validation-Completed Validation passed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants