-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
handling maven path space #754
base: master
Are you sure you want to change the base?
handling maven path space #754
Conversation
@@ -1181,7 +1181,7 @@ export const createJavaBom = async (path, options) => { | |||
`maven settings.xml found in ${basePath}. Please set the MVN_ARGS environment variable based on the full mvn build command used for this project.\nExample: MVN_ARGS='--settings ${settingsXml}'` | |||
); | |||
} | |||
const mavenCmd = getMavenCommand(basePath, path); | |||
const mavenCmd = '"'+ getMavenCommand(basePath, path)+'"'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @pandiyan-securin. I am not convinced if this is going to be enough to support the path with spaces on Windows fully.
I suspect the real issue could be the use of a single backslash instead of a double backslash.
C:\\Program Files\\Apache Maven\\apache-maven-3.8.4-bin\\mvn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prabhu , I have checked with double back slashes, still I am getting same kind of issue, Please refer the attached image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is the value specified? Via env args? Can you escape space with a single backslash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is passed via env args and did some hardcoded for testing purpose.
As well path.resolve()
returns path with single slash for os portable.
A single backslash () cannot escape a space.
Here are some additional notes from ChatGPT:
- Double backslashes are usually used in Windows paths, while single backslashes are more common in other operating systems.
- Using double backslashes can make our code less portable, as it may not work on all platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update:
Adding maven path to system environment variable is working fine.
When we add a environmental variable MAVEN_HOME for user specific then it throws above error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for looking into this. We can keep the issue open and add support for space (and UNC path) for Windows at some point for all languages and package managers. For now, people can use workarounds such as not using space in the directory names or using WSL, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For your kind information, When maven path has a space then this issue occurs even in WSL.
No description provided.