blackducksoftware / hub-detect

This is now deprecated. Please see synopsys-detect.
Apache License 2.0
38 stars 39 forks source link

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

Open ilatypov opened 6 years ago

ilatypov commented 6 years ago

Pull Request template

Link to github issue (if applicable):

Changes proposed in this pull request:

coveralls commented 6 years ago

Coverage Status

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

ekerwin commented 6 years ago

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 commented 6 years ago

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 commented 5 years ago

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

aschrab commented 5 years ago

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 commented 5 years ago

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)