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 from Brew #743

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

Conversation

dwalluck
Copy link
Collaborator

@dwalluck dwalluck commented Dec 5, 2024

No description provided.

@dwalluck dwalluck force-pushed the windows-artifact branch 4 times, most recently from c377cc3 to 87ccc26 Compare December 6, 2024 19:59
ArtifactBuilder<?, ?> builder;

switch (buildType) {
case GRADLE, MAVEN, SBT -> builder = createMavenArtifact(archiveInfo);
case NPM -> builder = createNpmArtifact(archiveInfo);
case maven -> builder = createMavenArtifact(archiveInfo);
Copy link
Contributor

Choose a reason for hiding this comment

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

what happened to the GRADLE and SBT type?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am adding a new Windows (WIN) build type to DelAn that already exists in kojiji in order to allow DelAn to scan zips that were created on a Windows host.

There never was a "gradle" or "sbt" type in Koji. This was a string field that was being "abused" in DelAn. I am changing the type to an enum in Kojiji to prevent this. See this PR release-engineering/kojiji#167.

Ultimately, I don't think that this PR changes anything in DelAn since it always called createMavenArtifact for the "gradle" or "sbt" types. The extra information will still be available somewhere else, but not in this field.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is made up of the two PRs above, plus project-ncl/pnc-api#244. The kojiji and pnc-api PRs need to go in first.

The current mapping from PNC to Brew is now if NPM then NPM else MAVEN.

Brew supports WIN, IMAGE and RPM types which aren't supported by PNC. The "gradle" and "sbt" types were PNC types not supported by Brew.

I am adding the WIN type to PNC (DelAn only). I don't see how the IMAGE type is useful, and the RPM type is the default type, and I am not sure that we scan RPMs directly with DelAn (although it's possible to do).

@janinko
Copy link
Contributor

janinko commented Dec 9, 2024

There is a lot of unnecessary unreleated code style changes. If you really want them, put them at least to different commit.

@dwalluck
Copy link
Collaborator Author

dwalluck commented Dec 9, 2024

There is a lot of unnecessary unreleated code style changes. If you really want them, put them at least to different commit.

Yes, I prefer to change to Java 17 style code now.

Sure, I will split it into a separate PR.

@dwalluck dwalluck force-pushed the windows-artifact branch 2 times, most recently from 497463f to d9637fa Compare December 18, 2024 14:38
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