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

New package: PlantUML.PlantUML version v1.2023.9 #110767

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

Conversation

seppeon
Copy link
Contributor

@seppeon seppeon commented Jun 27, 2023

I've made a manifest for PlantUML, which at this time does not work. I need some assistance fixing an issue.

  • Have you signed the Contributor License Agreement?
  • Have you checked that there aren't other open pull requests for the same manifest update/change?
  • This PR only modifies one (1) manifest
  • 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.4 schema?

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


Microsoft Reviewers: codeflow:open?pullrequest=https://github.com/microsoft/winget-pkgs/pull/110767&drop=dogfoodAlpha

@wingetbot
Copy link
Collaborator

Service Badge  Service Badge  

@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wingetbot wingetbot added the Manifest-Validation-Error Manifest validation failed label Jun 27, 2023
@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Manifest-Validation-Error Manifest validation failed label Jun 27, 2023
@seppeon
Copy link
Contributor Author

seppeon commented Jun 27, 2023

I need some help with this one:

  1. There is an issue where the standalone .jar file is renamed to .exe.
  2. I'd like to add a dependency to the OpenJDK or java runtime if possible.

@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wingetbot wingetbot added Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Validation-Executable-Error labels Jun 27, 2023
@wingetbot
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@microsoft-github-policy-service microsoft-github-policy-service bot removed Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Validation-Executable-Error labels Jun 27, 2023
@wingetbot wingetbot added Azure-Pipeline-Passed Validation pipeline passed. There may still be manual validation requirements. Validation-Executable-Error labels Jun 27, 2023
@stephengillie
Copy link
Collaborator

##[warning] Validation concluded that this task needs to go to manual review.

@stephengillie
Copy link
Collaborator

Manual Validation failed with:

1:55:31 PM: C:\Users\User\AppData\Local\Microsoft\WinGet\Links\plantuml.jar.exe
start-process : This command cannot be run due to the error: The specified executable is not a valid application for this OS platform..

@stephengillie
Copy link
Collaborator

This appears to be a portable JAR file.

  • Should Orcale.JavaRuntimeEdition or similar be a dependency?
  • The portable InstallerType appears to be appending a .exe to the file name. Another InstallerType might be needed, though I'm unsure of a better type.

@stephengillie stephengillie added the Needs-Author-Feedback This needs a response from the author. label Jun 27, 2023
@seppeon
Copy link
Contributor Author

seppeon commented Jun 28, 2023

I think yeah, Orcale.JavaRuntimeEdition seems sensible.

Thanks, yeah I'd have to bounce it back to the original author in that case. Is there a downside adding support for .jar in winget-cli, I have read through the code, it doesn't appear to be particularly complicated. There is a renaming function that performs a rename operation on the filenames that appear in the URL, it assumes anything standalone is .exe (and renames the extension with that assumption), remove that assumption and I believe it may work. I got that code building, but for some reason it ran the unmodified version lol, so I'll have to look at it again tonight.

The lines in question for winget-cli are in DownloadFlow.cpp, in the internally linked function called GetInstallerFileExtension.

@seppeon
Copy link
Contributor Author

seppeon commented Jun 28, 2023

Ok so I've put this PR up.
microsoft/winget-cli#3385

I think that might resolve the renaming issue. My main issue now is, I have no idea where wingetdev saves its files so I cannot check if its working.

@denelon
Copy link
Contributor

denelon commented Jun 28, 2023

As raised in:

I think the best thing to do here is raise a more explicit error when the portable package doesn't have the .exe extension.

We're going to have more work to do around reasoning how WinGet should handle a Java Archive as a "portable" package.

@denelon
Copy link
Contributor

denelon commented Jun 28, 2023

@stephengillie stephengillie removed the Needs-Attention This work item needs to be reviewed by a member of the core team. label Jun 28, 2023
@stephengillie stephengillie added Blocking-Issue Manifest validation is blocked by a known issue. portable-jar Portable JAR that needs JRE/JDK and to not be renamed. labels Jun 28, 2023
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. Blocking-Issue Manifest validation is blocked by a known issue. portable-jar Portable JAR that needs JRE/JDK and to not be renamed. Validation-Executable-Error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants