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

Added quotes around ${JAVA_PATH} in liquibase shell script to avoid spaces in path issues #1062

Merged
merged 3 commits into from
Dec 15, 2020

Conversation

szandany
Copy link
Contributor

@szandany szandany commented Mar 28, 2020

Branch: Liquibase/master

Added quotes around ${JAVA_PATH} in line 77 to avoid issues when there are spaces in the JAVA_PATH when using Emulators like "Git bash for Windows".

This fix will prevent having issues with spaces in JAVA_PATH. For example: when running Liquibase commands in Windows with Linux/Unix like Emulators such as: "Git bash for Windows" or "Minimalist GNU for Windows" with preexisting spaces in the JAVA_PATH.

┆Issue is synchronized with this Jira Bug by Unito
┆fixVersions: Community 4.3.0,Liquibase 4.3.0

…e are spaces in the JAVA_PATH when using Emulators like "Git bash for Windows".

This fix will prevent having issues with spaces in JAVA_PATH, when running Liquibase commands in Windows with Linux/Unix like Emulators such as: "Git bash for Windows" or "Minimalist GNU for Windows".
@SteveDonie
Copy link
Contributor

Thanks for your pull request! The team will review this and add comments with a status update soon.

Can you supply details about how this has been tested so far?

@szandany
Copy link
Contributor Author

how this has been tested so far?
I tested this on a Windows machine that already has Liquibase installed and the JAVA_PATH set with spaces in the path. For example, in CMD: which java /c/Program Files (x86)/Common Files/Oracle/Java/javapath/java
Then, I ran in git bash: liquibase --version and got the following error:
/c/apps/liquibase/liquibase: line 77: C:\Program: No such file or directory
After applying the fix in this pull request (Added quotes around ${JAVA_PATH} in liquibase shell script), I got it resolved.

@mariochampion
Copy link
Contributor

hey @szandany -- thanks for the update. Do you want this PR into master or 3.8.x?

@szandany
Copy link
Contributor Author

hey @szandany -- thanks for the update. Do you want this PR into master or 3.8.x?
No problem. I'm not sure what is the best choice here (master or 3.8.x).

@mariochampion
Copy link
Contributor

Well, the next 3.8.x release will come before the next Master release (altho likely by only a few weeks.) If you redirect this to a 3.8.x branch, we can see if it makes it in before next releases heads into final testing.

I would choose 3.8.x, but up to you @szandany

@codecov
Copy link

codecov bot commented Mar 31, 2020

Codecov Report

Merging #1062 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1062   +/-   ##
=========================================
  Coverage     47.71%   47.71%           
- Complexity     7476     7477    +1     
=========================================
  Files           757      757           
  Lines         36278    36278           
  Branches       6624     6624           
=========================================
+ Hits          17310    17311    +1     
  Misses        16660    16660           
+ Partials       2308     2307    -1     
Impacted Files Coverage Δ Complexity Δ
...re/src/main/java/liquibase/util/ISODateFormat.java 79.16% <0.00%> (+2.08%) 16.00% <0.00%> (+1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cec0a9...1eaa1c9. Read the comment docs.

@SteveDonie SteveDonie added the RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. label Apr 3, 2020
@molivasdat molivasdat changed the base branch from master to 4.3.x December 15, 2020 22:00
@molivasdat molivasdat changed the base branch from 4.3.x to master December 15, 2020 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DBAll IntegrationCLI RiskLow Trivial changes in spelling, documentation changes, focused bug fixes, etc. ThemeUsability TypeBug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants