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

Add support for Windows artifacts to Deliverables Analyzer #4137

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dwalluck
Copy link
Contributor

@dwalluck dwalluck commented Dec 9, 2024

Checklist:

  • Have you added unit tests for your change?

@dwalluck dwalluck requested a review from janinko December 9, 2024 21:02
purlBuilder = PackageURLBuilder.aPackageURL()
.withType(PackageURL.StandardTypes.GENERIC)
.withName(windowsArtifact.getName())
.withVersion(windowsArtifact.getVersion());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pkocandr Should the namespace be empty? Type is GENERIC as there is no WINDOWS type.

.collect(Collectors.joining(":"));
} else if (artifact instanceof WindowsArtifact) {
WindowsArtifact windowsArtifact = (WindowsArtifact) artifact;
return String.join("-", windowsArtifact.getName(), windowsArtifact.getVersion());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pkocandr For Windows, I just chose <name>-<version> as the identifier. I wonder if it's similar for NPM. It obviously will not match the Maven identifier format.

deployPath = "/" + mavenArtifact.getGroupId().replace('.', '/') + "/" + mavenArtifact.getArtifactId() + "/"
+ mavenArtifact.getVersion() + "/" + filename;
} else if (artifact instanceof WindowsArtifact) {
deployPath = "/" + filename;
Copy link
Contributor

@janinko janinko Dec 17, 2024

Choose a reason for hiding this comment

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

@pkocandr I am not sure how the deploy path would be if we got that artifact via brew pull (is it even possible?) and if this matches.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I understand the code, this isn't the Maven repository path, e.g., /maven2. This is the original Brew path (see createBrewOriginURL()).

Maven artifacts start with /maven and Windows artifacts start with /win, so I think it just uses the artifact type to start the path. For Windows, since there is no group identifier, etc., the rest of the path is just the filename itself.

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.

2 participants