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

osfile Macro does not create system specifc paths #5364

Closed
juergen-albert opened this issue Sep 1, 2022 · 4 comments · Fixed by #5366
Closed

osfile Macro does not create system specifc paths #5364

juergen-albert opened this issue Sep 1, 2022 · 4 comments · Fixed by #5366
Assignees
Milestone

Comments

@juergen-albert
Copy link
Contributor

return IO.absolutePath(f);

I'm on Windows and I would expect that ${osfile;test;atest} would produce a path with \ in it, but in IO.absolutPath we finally do this:

return path.replace(File.separatorChar, '/');

Which makes the whole thing do the opposite of what it should.

@bjhargrave
Copy link
Member

In Bnd all paths are forward slash so when macros process them, they don't need to worry.

Windows accepts forward slash in files names, except for the command shell. So we do quoting on windows to handle paths with forward slash.

if (IO.isWindows()) {
// [cs] Arguments on windows aren't processed correctly.
// So we need to perform some escaping.
// http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6511002
// http://msdn.microsoft.com/en-us/library/17w5ykft.aspx
p.command(p.command()
.stream()
.map(Command::windowsQuote)
.collect(toList()));
}

So, Bnd always uses forward slashed in paths.

@juergen-albert
Copy link
Contributor Author

Hmm, this does not seem to work for everything. If I try e.g. ${system;mkdir ${osfile;test;atest}} the result is something like mkdir n:/someworkspace/project/test/atest and mkdir complains that the directory is in the wrong format. Using the mkdir n:\someworkspace\project\test\atest however works. The command might be a special case though as other commands work as you say.

@bjhargrave
Copy link
Member

I think you are correct here for osfile as its purpose appears to be making os specific paths. c7f942b from 4 years ago is the where I changed this. Let me make a PR to change this back.

@bjhargrave bjhargrave self-assigned this Sep 2, 2022
@bjhargrave bjhargrave added this to the 6.4 milestone Sep 2, 2022
@juergen-albert
Copy link
Contributor Author

Thanks heaps!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants