Preserve multi-word parameters. Use TEMP and JAVA_HOME. #299

ilatypov · 2018-07-13T18:51:24Z

Pull Request template

Link to github issue (if applicable):

Changes proposed in this pull request:

Preserve arrays.
Use TEMP and JAVA_HOME.
Convert TEMP and JAVA_HOME to the native and POSIX format, respectively, on Cygwin, for use by the native Java and POSIX Bash, respectively.

…VA_HOME.

coveralls · 2018-07-13T18:56:26Z

Coverage decreased (-3.3%) to 23.046% when pulling 9a2da34 on ilatypov:master into 2e3cc15 on blackducksoftware:master.

jamesrichard91 · 2018-07-13T20:53:00Z

hub-detect/src/main/resources/hub-detect-sh

  else
    if [ -z "${DETECT_RELEASE_VERSION}" ]; then
-      DETECT_RELEASE_VERSION=$(curl $DETECT_CURL_OPTS 'https://test-repo.blackducksoftware.com/artifactory/api/search/latestVersion?g=com.blackducksoftware.integration&a=hub-detect&repos=bds-integrations-release')
+      DETECT_RELEASE_VERSION=$(curl -sSL $DETECT_CURL_OPTS 'https://test-repo.blackducksoftware.com/artifactory/api/search/latestVersion?g=com.blackducksoftware.integration&a=hub-detect&repos=bds-integrations-release')


looking up the Curl man page, I think this is supposed to be -ssl not -sSL. Could you change these?

Each letter in the -XYZ notation is a separate option. I added the lower-case "s" to hide the progress screen and an upper-case S to show error messages such as Zscaler certificate errors.

My employer has a ticket 00085307 on specifying a space-separated value in one of the parameters,

--detect.maven.build.command="compile -s settings.xml"

The error comes from Travis with regard to submitting Sonar analysis.

The existing SonarQube server may be correct. The SONAR_HOST_URL variable had no value.

The error said, > You're only authorized to execute a local (preview) SonarQube analysis without pushing the results to the SonarQube server. Please contact your SonarQube administrator. I just reverted my attempts to work this around now that I logged in and authorized sonarcloud.io.

Follow Travis documentation implying that pull requests from forks are not to be inspected with SonarCloud. https://docs.travis-ci.com/user/sonarcloud/ Attempt to classify the secure token as belonging to a certain organization so that a pull request from my fork does not get a SonarCloud inspection.

Get rid of SonarCloud as I fail to prevent its integration failure in my pull request from a fork. https://stackoverflow.com/questions/45612758/how-do-i-get-sonarcloud-to-run-on-pull-requests-from-forks-with-travis-maven https://travis-ci.org/blackducksoftware/hub-detect/builds/413657598 Maintainers, please re-add "sonarqube" to the script command line after my requests pulls through.

ekerwin · 2018-08-09T18:43:13Z

Changes to the shell script right now are concerning because they aren't tied to a release and are immediately put into customer's hands. This is changing soon so we'll be able to more safely introduce changes to the shell script.

This method of preserving multi-word parameters is much better than what we original did since our implementation demanded the 'rm' command be used to clean up the file we create and a customer just experienced problems with that.

In the fullness of time, we should be able to merge this PR to change the shell script as soon as we can change and test it - I'm hoping for detect 4.3.0 (sometime in the early fall of 2018).

DanaMaxfield · 2018-08-13T16:30:26Z

hub-detect/src/main/resources/hub-detect-sh

-      echo "will look for snapshot: hub-detect-latest-SNAPSHOT.jar"
      DETECT_SOURCE="https://test-repo.blackducksoftware.com/artifactory/bds-integrations-snapshot/com/blackducksoftware/integration/hub-detect/latest-SNAPSHOT/hub-detect-latest-SNAPSHOT.jar"
      DETECT_DESTINATION="${DETECT_JAR_PATH}/hub-detect-latest-SNAPSHOT.jar"
+      echo "will look for snapshot: ${DETECT_SOURCE}" >&2


All of the edits with redirects to stderr in here are info type of messages, not errors. My opinion is that they should be sent to stdout.

Thanks for stipulating that. I dropped the ball after seeing no change in production concerning the bug itself (as opposed to following the diagnostic conventions).

DanaMaxfield · 2018-08-13T16:30:51Z

hub-detect/src/main/resources/hub-detect-sh

-  rm -f $DETECT_JAR_PATH/hub-detect-java.sh
-  exit $RESULT
+  javacmd=(java ${DETECT_JAVA_OPTS} -jar "${DETECT_DESTINATION}")
+  type -a java || return -1


Why 'type -a' vs just type? It feels that using the '-a' option adds more noise than is necessary and could be misleading to someone trying to troubleshoot.
I would also move the check of java to the top after the handling of JAVA_HOME.

I am surprised I did not redirect stderr to /dev/null here. The -a option has an unfortunate double meaning in checking for all possible executable commands (not just files in PATH) and in showing them.

DanaMaxfield · 2018-08-13T16:31:04Z

hub-detect/src/main/resources/hub-detect-sh

+  javacmd=(java ${DETECT_JAVA_OPTS} -jar "${DETECT_DESTINATION}")
+  type -a java || return -1
+  echo "running detect: ${javacmd[*]@Q} ${loggable_script_args[*]@Q}" >&2
+  "${javacmd[@]}" "${script_args[@]}" || return $?


Is it a valid condition to have special characters within these 2 variables which require the need to print them with '@q'?

The @Q operator quotes array elements, according to the new "parameter transformation" man paragraph,

https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html

DanaMaxfield · 2018-08-13T16:33:41Z

Should the original script use “set -x”? Can’t possible check/test for everything, so using set -x can be a good catch all.
Curl can fail due to a bad domain, filename, network, etc; but the curl executable will still be successful. Can we implement the usage of getting and checking the http_code (-w "%{http_code}”). When I test this using good and bad data, the return status is always 0, but the http_code will be 200 for successful commands. My output:

$ ./testCurl.sh
Bad name status: 404 : return status: 0
Bad domain status: 404 : return status: 0
Valid file and domain status: 200 : return status: 0

nwinkler · 2019-02-04T08:47:17Z

Any updates on this? When can this be integrated? (Asking since I've run into the same issue...)

aschrab · 2019-02-11T17:55:37Z

Should the original script use “set -x”? Can’t possible check/test for everything, so using set -x can be a good catch all.

Did you mean set -e? set -x would just show what's being done.

JakeMathews · 2019-06-07T15:05:47Z

The scripts have moved to https://github.com/synopsys-sig/synopsys-detect-scripts and changes similar to these should be made available with the release of 5.5.0 of Synopsys-Detect (https://github.com/blackducksoftware/synopsys-detect)

Preserve multi-word parameters blackducksoftware#298. Use TEMP and JA…

1347ec7

…VA_HOME.

ilatypov changed the title ~~Preserve multi-word parameters #298. Use TEMP and JAVA_HOME.~~ Preserve multi-word parameters. Use TEMP and JAVA_HOME. Jul 13, 2018

jamesrichard91 reviewed Jul 13, 2018

View reviewed changes

ilatypov added 5 commits August 8, 2018 11:28

Attempt to work around "You are not authorized"

d324792

The error comes from Travis with regard to submitting Sonar analysis.

Attempt to work around "You are not authorized"

5d19059

The existing SonarQube server may be correct. The SONAR_HOST_URL variable had no value.

DanaMaxfield reviewed Aug 13, 2018

View reviewed changes

Preserve multi-word parameters. Use TEMP and JAVA_HOME. #299

Are you sure you want to change the base?

Preserve multi-word parameters. Use TEMP and JAVA_HOME. #299

Uh oh!

Conversation

ilatypov commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request template

Uh oh!

coveralls commented Jul 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jamesrichard91 Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

ilatypov Jul 14, 2018

Choose a reason for hiding this comment

Uh oh!

ekerwin commented Aug 9, 2018

Uh oh!

DanaMaxfield Aug 13, 2018

Choose a reason for hiding this comment

Uh oh!

ilatypov Nov 26, 2018

Choose a reason for hiding this comment

Uh oh!

DanaMaxfield Aug 13, 2018

Choose a reason for hiding this comment

Uh oh!

ilatypov Nov 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanaMaxfield Aug 13, 2018

Choose a reason for hiding this comment

Uh oh!

ilatypov Nov 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanaMaxfield commented Aug 13, 2018

Uh oh!

nwinkler commented Feb 4, 2019

Uh oh!

aschrab commented Feb 11, 2019

Uh oh!

JakeMathews commented Jun 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ilatypov commented Jul 13, 2018 •

edited

Loading

coveralls commented Jul 13, 2018 •

edited

Loading

ilatypov Nov 26, 2018 •

edited

Loading

ilatypov Nov 26, 2018 •

edited

Loading

JakeMathews commented Jun 7, 2019 •

edited

Loading