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.

Sign up for GitHub

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

Jump to bottom

Fix npm publish NPE #1307

Merged
merged 3 commits into from
Nov 24, 2021
Merged

Fix npm publish NPE #1307

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7243e20
Fix npm NPE
RobiNino Nov 24, 2021
895714b
Update deprecated warning syntax
RobiNino Nov 24, 2021
1065d83
CR comments
RobiNino Nov 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions artifactory/cli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("mvnc", utils.Maven, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("mvnc", "rt", utils.Maven, c, cliutils.CreateConfigCmd)
},
},
{
Expand All @@ -377,7 +377,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("gradlec", utils.Gradle, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("gradlec", "rt", utils.Gradle, c, cliutils.CreateConfigCmd)
},
},
{
Expand Down Expand Up @@ -493,7 +493,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("npmc", utils.Npm, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("npmc", "rt", utils.Npm, c, cliutils.CreateConfigCmd)
},
},
{
Expand Down Expand Up @@ -534,7 +534,7 @@ func GetCommands() []cli.Command {
SkipFlagParsing: true,
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return npmPublishCmd(c)
return cliutils.RunNativeCmdWithDeprecationWarning("npm p", utils.Npm, c, npmPublishCmd)
},
},
{
Expand All @@ -546,7 +546,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("yarnc", utils.Yarn, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("yarnc", "rt", utils.Yarn, c, cliutils.CreateConfigCmd)
},
},
{
Expand All @@ -570,7 +570,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("nugetc", utils.Nuget, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("nugetc", "rt", utils.Nuget, c, cliutils.CreateConfigCmd)
},
},
{
Expand Down Expand Up @@ -606,7 +606,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("dotnetc", utils.Dotnet, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("dotnetc", "rt", utils.Dotnet, c, cliutils.CreateConfigCmd)
},
},
{
Expand All @@ -630,7 +630,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("go-config", utils.Go, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("go-config", "rt", utils.Go, c, cliutils.CreateConfigCmd)
},
},
{
Expand All @@ -643,7 +643,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunCmdWithDeprecationWarning("gp", c, buildtools.GoPublishCmd)
return cliutils.RunCmdWithDeprecationWarning("gp", "rt", c, buildtools.GoPublishCmd)
},
},
{
Expand Down Expand Up @@ -695,7 +695,7 @@ func GetCommands() []cli.Command {
ArgsUsage: common.CreateEnvVars(),
BashComplete: corecommon.CreateBashCompletionFunc(),
Action: func(c *cli.Context) error {
return cliutils.RunConfigCmdWithDeprecationWarning("pipc", utils.Pip, c, cliutils.CreateConfigCmd)
return cliutils.RunConfigCmdWithDeprecationWarning("pipc", "rt", utils.Pip, c, cliutils.CreateConfigCmd)
},
},
{
Expand Down Expand Up @@ -1179,6 +1179,7 @@ func npmDeprecatedInstallCiCmd(c *cli.Context, npmCmd *npm.NpmInstallOrCiCommand
return commands.Exec(npmCmd)
}

// Deprecated
func npmPublishCmd(c *cli.Context) error {
if show, err := cliutils.ShowCmdHelpIfNeeded(c, c.Args()); show || err != nil {
return err
Expand All @@ -1194,6 +1195,10 @@ func npmPublishCmd(c *cli.Context) error {
args := cliutils.ExtractCommand(c)
npmCmd := npm.NewNpmPublishCommand()
npmCmd.SetConfigFilePath(configFilePath).SetArgs(args)
err = npmCmd.Init()
if err != nil {
return err
}
err = commands.Exec(npmCmd)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ require (

//replace github.com/jfrog/jfrog-client-go => github.com/jfrog/jfrog-client-go v1.6.1-0.20211118142316-d9a261c51de9

//replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.4.3-0.20211118181121-5137c711021e
replace github.com/jfrog/jfrog-cli-core/v2 => github.com/RobiNino/jfrog-cli-core/v2 v2.0.0-20211124102537-7c0290295327

//replace github.com/jfrog/gocmd => github.com/jfrog/gocmd v0.5.4-0.20211116142418-92ddf344a6a4

Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ github.com/Microsoft/go-winio v0.4.16 h1:FtSW/jqD+l4ba5iPBj9CODVtgfYAD8w2wS923g/
github.com/Microsoft/go-winio v0.4.16/go.mod h1:XB6nPKklQyQ7GC9LdcBEcBl8PF76WugXOPRXwdLnMv0=
github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7 h1:YoJbenK9C67SkzkDfmQuVln04ygHj3vjZfd9FL+GmQQ=
github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7/go.mod h1:z4/9nQmJSSwwds7ejkxaJwO37dru3geImFUdJlaLzQo=
github.com/RobiNino/jfrog-cli-core/v2 v2.0.0-20211124102537-7c0290295327 h1:8L6Lcrgzg1p8qabtO01Byx43guE4aPR5C1lOuoW/0/4=
github.com/RobiNino/jfrog-cli-core/v2 v2.0.0-20211124102537-7c0290295327/go.mod h1:DTkvS806Vn2aF23ng/ZcEXFpL0rIsVN6AMNsXY3nr0A=
github.com/VividCortex/ewma v1.1.1 h1:MnEK4VOv6n0RSY4vtRe3h11qjxL3+t0B8yOL8iMXdcM=
github.com/VividCortex/ewma v1.1.1/go.mod h1:2Tkkvm3sRDVXaiyucHiACn4cqf7DpdyLvmxzcbUokwA=
github.com/acomagu/bufpipe v1.0.3 h1:fxAGrHZTgQ9w5QqVItgzwj235/uYZYgbXitB+dLupOk=
Expand Down Expand Up @@ -229,8 +231,6 @@ github.com/jfrog/gocmd v0.5.4 h1:+yrZ6e3HZXJf6knORZxfbF/LCsK7FTdzmxHPbAaJtQk=
github.com/jfrog/gocmd v0.5.4/go.mod h1:nASNY/3Rv7M9VHjim+pIowo0I/fd38SJJm/Bxcp/Tww=
github.com/jfrog/gofrog v1.1.0 h1:nhbfSVUYWRxKHeU0RsPAUQeo1suc4oAcEttIKQRvWhs=
github.com/jfrog/gofrog v1.1.0/go.mod h1:9YN5v4LlsCfLIXpwQnzSf1wVtgjdHM20FzuIu58RMI4=
github.com/jfrog/jfrog-cli-core/v2 v2.5.0 h1:s3/Lzm4NScRgStkdohC+FMcmPeBrm2GFgdIl+Q2QNsQ=
github.com/jfrog/jfrog-cli-core/v2 v2.5.0/go.mod h1:DTkvS806Vn2aF23ng/ZcEXFpL0rIsVN6AMNsXY3nr0A=
github.com/jfrog/jfrog-client-go v1.6.1 h1:uu1kr1pOHb0NmT+Equm+FwOOoksWCeSqBOdhmd+60HY=
github.com/jfrog/jfrog-client-go v1.6.1/go.mod h1:oR5emUgDSN+T8jNXqimKy9VxszQQuWumv1bst8od3vU=
github.com/json-iterator/go v1.1.11/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4=
Expand Down
46 changes: 29 additions & 17 deletions npm_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ import (

type npmTestParams struct {
testName string
command string
nativeCommand string
legacyCommand string
RobiNino marked this conversation as resolved.
Show resolved Hide resolved
repo string
npmArgs string
wd string
Expand All @@ -47,7 +48,15 @@ func cleanNpmTest() {
tests.CleanFileSystem()
}

func TestNpm(t *testing.T) {
func TestNpmNativeSyntax(t *testing.T) {
testNpm(t, false)
}

func TestNpmLegacy(t *testing.T) {
testNpm(t, true)
}

func testNpm(t *testing.T, isLegacy bool) {
initNpmTest(t)
defer cleanNpmTest()
wd, err := os.Getwd()
Expand All @@ -67,28 +76,32 @@ func TestNpm(t *testing.T) {

npmProjectPath, npmScopedProjectPath, npmNpmrcProjectPath, npmProjectCi := initNpmFilesTest(t)
var npmTests = []npmTestParams{
{testName: "npm ci", command: "npm ci", repo: tests.NpmRemoteRepo, wd: npmProjectCi, validationFunc: validateNpmInstall},
{testName: "npm ci with module", command: "npm ci", repo: tests.NpmRemoteRepo, wd: npmProjectCi, moduleName: ModuleNameJFrogTest, validationFunc: validateNpmInstall},
{testName: "npm i with module", command: "npm install", repo: tests.NpmRemoteRepo, wd: npmProjectPath, moduleName: ModuleNameJFrogTest, validationFunc: validateNpmInstall},
{testName: "npm i with scoped project", command: "npm install", repo: tests.NpmRemoteRepo, wd: npmScopedProjectPath, validationFunc: validateNpmInstall},
{testName: "npm i with npmrc project", command: "npm install", repo: tests.NpmRemoteRepo, wd: npmNpmrcProjectPath, validationFunc: validateNpmInstall},
{testName: "npm i with production", command: "npm install", repo: tests.NpmRemoteRepo, wd: npmProjectPath, validationFunc: validateNpmInstall, npmArgs: "--production"},
{testName: "npm i with npmrc project", command: "npm i", repo: tests.NpmRemoteRepo, wd: npmNpmrcProjectPath, validationFunc: validateNpmPackInstall, npmArgs: "yaml"},
{testName: "npmp with module", command: "npm p", repo: tests.NpmRepo, wd: npmScopedProjectPath, moduleName: ModuleNameJFrogTest, validationFunc: validateNpmScopedPublish},
{testName: "npmp", command: "npm publish", repo: tests.NpmRepo, wd: npmProjectPath, validationFunc: validateNpmPublish},
{testName: "npm conditional publish", command: "npm publish --scan", repo: tests.NpmRepo, wd: npmProjectPath, validationFunc: validateNpmPublish},
{testName: "npm ci", nativeCommand: "npm ci", legacyCommand: "rt npmci", repo: tests.NpmRemoteRepo, wd: npmProjectCi, validationFunc: validateNpmInstall},
{testName: "npm ci with module", nativeCommand: "npm ci", legacyCommand: "rt npmci", repo: tests.NpmRemoteRepo, wd: npmProjectCi, moduleName: ModuleNameJFrogTest, validationFunc: validateNpmInstall},
{testName: "npm i with module", nativeCommand: "npm install", legacyCommand: "rt npm-install", repo: tests.NpmRemoteRepo, wd: npmProjectPath, moduleName: ModuleNameJFrogTest, validationFunc: validateNpmInstall},
{testName: "npm i with scoped project", nativeCommand: "npm install", legacyCommand: "rt npm-install", repo: tests.NpmRemoteRepo, wd: npmScopedProjectPath, validationFunc: validateNpmInstall},
{testName: "npm i with npmrc project", nativeCommand: "npm install", legacyCommand: "rt npm-install", repo: tests.NpmRemoteRepo, wd: npmNpmrcProjectPath, validationFunc: validateNpmInstall},
{testName: "npm i with production", nativeCommand: "npm install", legacyCommand: "rt npm-install", repo: tests.NpmRemoteRepo, wd: npmProjectPath, validationFunc: validateNpmInstall, npmArgs: "--production"},
{testName: "npm i with npmrc project", nativeCommand: "npm i", legacyCommand: "rt npmi", repo: tests.NpmRemoteRepo, wd: npmNpmrcProjectPath, validationFunc: validateNpmPackInstall, npmArgs: "yaml"},
{testName: "npm p with module", nativeCommand: "npm p", legacyCommand: "rt npmp", repo: tests.NpmRepo, wd: npmScopedProjectPath, moduleName: ModuleNameJFrogTest, validationFunc: validateNpmScopedPublish},
{testName: "npm p", nativeCommand: "npm publish", legacyCommand: "rt npm-publish", repo: tests.NpmRepo, wd: npmProjectPath, validationFunc: validateNpmPublish},
{testName: "npm conditional publish", nativeCommand: "npm publish --scan", legacyCommand: "rt npm-publish --scan", repo: tests.NpmRepo, wd: npmProjectPath, validationFunc: validateNpmPublish},
}

for i, npmTest := range npmTests {
t.Run(npmTest.testName, func(t *testing.T) {
npmCmd := npmTest.nativeCommand
if isLegacy {
npmCmd = npmTest.legacyCommand
}
err = os.Chdir(filepath.Dir(npmTest.wd))
assert.NoError(t, err)
npmrcFileInfo, err := os.Stat(".npmrc")
if err != nil && !os.IsNotExist(err) {
assert.Fail(t, err.Error())
}
var buildNumber string
commandArgs := strings.Split(npmTest.command, " ")
commandArgs := strings.Split(npmCmd, " ")
buildNumber = strconv.Itoa(i + 100)
commandArgs = append(commandArgs, npmTest.npmArgs)

Expand Down Expand Up @@ -231,12 +244,11 @@ func validateNpmInstall(t *testing.T, npmTestParams npmTestParams, isNpm7 bool)
return
}
buildInfo := publishedBuildInfo.BuildInfo
if buildInfo.Modules == nil || len(buildInfo.Modules) == 0 {
// Case no module was created
t.Error(fmt.Sprintf("npm install test with command '%s' and repo '%s' failed", npmTestParams.command, npmTestParams.repo))
if buildInfo.Modules == nil {
assert.NotNil(t, buildInfo.Modules)
return
}

assert.NotEmpty(t, buildInfo.Modules)
equalDependenciesSlices(t, expectedDependencies, buildInfo.Modules[0].Dependencies)
}

Expand Down
27 changes: 20 additions & 7 deletions pip_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ type PipCmd struct {
Options []string
}

func TestPipInstall(t *testing.T) {
func TestPipInstallNativeSyntax(t *testing.T) {
testPipInstall(t, false)
}

func TestPipInstallLegacy(t *testing.T) {
RobiNino marked this conversation as resolved.
Show resolved Hide resolved
testPipInstall(t, true)
}

func testPipInstall(t *testing.T, isLegacy bool) {
// Init pip.
initPipTest(t)

Expand Down Expand Up @@ -53,17 +61,22 @@ func TestPipInstall(t *testing.T) {
expectedDependencies int
cleanAfterExecution bool
}{
{"setuppy", "setuppyproject", "setuppy", "jfrog-python-example", []string{"pip", "install", ".", "--no-cache-dir", "--force-reinstall", "--build-name=" + tests.PipBuildName}, 3, true},
{"setuppy-verbose", "setuppyproject", "setuppy-verbose", "jfrog-python-example", []string{"pip", "install", ".", "--no-cache-dir", "--force-reinstall", "-v", "--build-name=" + tests.PipBuildName}, 3, true},
{"setuppy-with-module", "setuppyproject", "setuppy-with-module", "setuppy-with-module", []string{"pip", "install", ".", "--no-cache-dir", "--force-reinstall", "--build-name=" + tests.PipBuildName, "--module=setuppy-with-module"}, 3, true},
{"requirements", "requirementsproject", "requirements", tests.PipBuildName, []string{"pip", "install", "-r", "requirements.txt", "--no-cache-dir", "--force-reinstall", "--build-name=" + tests.PipBuildName}, 5, true},
{"requirements-verbose", "requirementsproject", "requirements-verbose", tests.PipBuildName, []string{"pip", "install", "-r", "requirements.txt", "--no-cache-dir", "--force-reinstall", "-v", "--build-name=" + tests.PipBuildName}, 5, false},
{"requirements-use-cache", "requirementsproject", "requirements-verbose", "requirements-verbose-use-cache", []string{"pip", "install", "-r", "requirements.txt", "--module=requirements-verbose-use-cache", "--build-name=" + tests.PipBuildName}, 5, true},
{"setuppy", "setuppyproject", "setuppy", "jfrog-python-example", []string{".", "--no-cache-dir", "--force-reinstall", "--build-name=" + tests.PipBuildName}, 3, true},
{"setuppy-verbose", "setuppyproject", "setuppy-verbose", "jfrog-python-example", []string{".", "--no-cache-dir", "--force-reinstall", "-v", "--build-name=" + tests.PipBuildName}, 3, true},
{"setuppy-with-module", "setuppyproject", "setuppy-with-module", "setuppy-with-module", []string{".", "--no-cache-dir", "--force-reinstall", "--build-name=" + tests.PipBuildName, "--module=setuppy-with-module"}, 3, true},
{"requirements", "requirementsproject", "requirements", tests.PipBuildName, []string{"-r", "requirements.txt", "--no-cache-dir", "--force-reinstall", "--build-name=" + tests.PipBuildName}, 5, true},
{"requirements-verbose", "requirementsproject", "requirements-verbose", tests.PipBuildName, []string{"-r", "requirements.txt", "--no-cache-dir", "--force-reinstall", "-v", "--build-name=" + tests.PipBuildName}, 5, false},
{"requirements-use-cache", "requirementsproject", "requirements-verbose", "requirements-verbose-use-cache", []string{"-r", "requirements.txt", "--module=requirements-verbose-use-cache", "--build-name=" + tests.PipBuildName}, 5, true},
}

// Run test cases.
for buildNumber, test := range allTests {
t.Run(test.name, func(t *testing.T) {
if isLegacy {
test.args = append([]string{"rt", "pip-install"}, test.args...)
} else {
test.args = append([]string{"pip", "install"}, test.args...)
}
testPipCmd(t, test.name, createPipProject(t, test.outputFolder, test.project), strconv.Itoa(buildNumber), test.moduleId, test.expectedDependencies, test.args)
if test.cleanAfterExecution {
// cleanup
Expand Down
29 changes: 18 additions & 11 deletions utils/cliutils/utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,30 +516,37 @@ func CreateConfigCmd(c *cli.Context, confType artifactoryUtils.ProjectType) erro

func RunNativeCmdWithDeprecationWarning(cmdName string, projectType artifactoryUtils.ProjectType, c *cli.Context, cmd func(c *cli.Context) error) error {
RobiNino marked this conversation as resolved.
Show resolved Hide resolved
if shouldLogWarning() {
log.Warn(`You are using a deprecated syntax of the command.
The new command syntax is quite similar to the current syntax, and is similar to the ` + projectType.String() + ` CLI command, with the addition of a prefix of the 'jf' executable name, i.e.:
$ jf ` + cmdName + ` [` + projectType.String() + ` args and option] --build-name=*BUILD_NAME* --build-number=*BUILD_NUMBER*`)
log.Warn(
`You are using a deprecated syntax of the command.
The new command syntax is quite similar to the syntax used by the native ` + projectType.String() + ` client.
All you need to do is to add '` + coreutils.GetCliExecutableName() + `' as a prefix to the command.
For example:
$ ` + coreutils.GetCliExecutableName() + ` ` + cmdName + ` ...
The --build-name and --build-number options are still supported.`)
}
return cmd(c)
}

func RunConfigCmdWithDeprecationWarning(cmdName string, confType artifactoryUtils.ProjectType, c *cli.Context,
func RunConfigCmdWithDeprecationWarning(cmdName, oldSubcommand string, confType artifactoryUtils.ProjectType, c *cli.Context,
cmd func(c *cli.Context, confType artifactoryUtils.ProjectType) error) error {
logNonNativeCommandDeprecation(cmdName)
logNonNativeCommandDeprecation(cmdName, oldSubcommand)
return cmd(c, confType)
}

func RunCmdWithDeprecationWarning(cmdName string, c *cli.Context,
func RunCmdWithDeprecationWarning(cmdName, oldSubcommand string, c *cli.Context,
cmd func(c *cli.Context) error) error {
logNonNativeCommandDeprecation(cmdName)
logNonNativeCommandDeprecation(cmdName, oldSubcommand)
return cmd(c)
}

func logNonNativeCommandDeprecation(cmdName string) {
func logNonNativeCommandDeprecation(cmdName, oldSubcommand string) {
if shouldLogWarning() {
log.Warn(`You are using a deprecated syntax of the command.
The new command syntax is similar to the current syntax, without the subcommand, i.e.:
$ jf ` + cmdName + ` [args and option]`)
log.Warn(
`You are using a deprecated syntax of the command.
Instead of:
$ ` + coreutils.GetCliExecutableName() + ` ` + oldSubcommand + ` ` + cmdName + ` ...
Use:
$ ` + coreutils.GetCliExecutableName() + ` ` + cmdName + ` ...`)
}
}

Expand Down
Loading