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

Fix root module name handling in build info collection #1201

Merged

Conversation

EyalDelarea
Copy link
Contributor

@EyalDelarea EyalDelarea commented Jul 15, 2024

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • This pull request is on the dev branch.
  • I used gofmt for formatting the code before submitting the pull request.

Bug Description:

When collecting build information for different packages, if the module name is not specified in the descriptor file or provided as a CLI argument, it defaults to an empty string. This results in an error from the Artifactory server, stating that the module ID cannot be null.

16:22:54 [Error] server response: 400 
{
  "errors": [
    {
      "status": 400,
      "message": "Module id name cannot be null!"
    }
  ]
}

In previous CLI versions, the default value was ":", which prevented crashes.

Fix:

If no module ID is provided, the default will now be set to the base directory of the module. This aligns with the default behavior of package descriptors init scripts.
As a last reasort the module name will resolve to "module" to be displayed at the UI and not fail on publish.

artifactory/commands/npm/npmcommand.go Outdated Show resolved Hide resolved
@EyalDelarea EyalDelarea changed the title Fix missing root module ID for npm Fix missing root module ID for build info Jul 15, 2024
@EyalDelarea EyalDelarea changed the title Fix missing root module ID for build info Fix missing root module ID for published build info Jul 15, 2024
@EyalDelarea EyalDelarea changed the title Fix missing root module ID for published build info Fix Default Module Name Handling in Build Info Collection Jul 15, 2024
@EyalDelarea EyalDelarea changed the title Fix Default Module Name Handling in Build Info Collection Fix default module name handling in build info collection Jul 15, 2024
@EyalDelarea EyalDelarea changed the title Fix default module name handling in build info collection Fix root module name handling in build info collection Jul 15, 2024
@EyalDelarea EyalDelarea force-pushed the 2366-module-id-missing-in-npm-build-info branch from 3c52e30 to 2e153b9 Compare July 15, 2024 14:55
@EyalDelarea
Copy link
Contributor Author

@yahavi i've changed the function to return error if fails to get working dir.
let me know what you think 🚀

Copy link
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


common/build/buildutils.go Outdated Show resolved Hide resolved
@RobiNino RobiNino merged commit 2df2041 into jfrog:dev Jul 30, 2024
6 of 7 checks passed
RobiNino added a commit to RobiNino/jfrog-cli-core that referenced this pull request Jul 30, 2024
@kunkka-m
Copy link

kunkka-m commented Aug 9, 2024

image image I used the latest version of jfrog-cli(2.63.0), but this issue still happened

@kunkka-m
Copy link

kunkka-m commented Aug 9, 2024

Oh, This MR is to dev branch!

@lincoohe
Copy link

Hi @EyalDelarea ,it should be the build-publish issue instead of bce issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants