Nested target feature needs to support windows path in project_loc expansion

[habbit-baggins]

This issue is related to the new feature of nested target files introduced by #401 . Both in Tycho and in PDE, when referring to a local file the syntax is supposed to use a file: URI for a local file, and relative paths are heavily discouraged because they refer to the actual CWD, not to the project/workspace root. Thus, the example project refers to the nested target file using the ${project_loc} Eclipse variable that expands to the absolute path of a project:

<location type="Target" uri="file:${project_loc:/target.refs}/base.target"/>

However, when trying to run this build on Windows, I get the following error:

PS C:\ws\target-testcase\tycho\target.references> mvn validate
[INFO] Scanning for projects...
[WARNING] No explicit target runtime environment configuration. Build is platform dependent.
[INFO] Computing target platform for MavenProject: org.eclipse.tycho.itests:test.bundle:0.0.1-SNAPSHOT @ C:\ws\target-testcase\tycho\target.references\test.bundle\pom.xml
[INFO] Resolving target definition file:/C:/ws/target-testcase/tycho/target.references/target.refs/target.refs.target for environments=[win32/win32/x86_64], include source mode=honor, execution environment=StandardEEResolutionHints [executionEnvironment=OSGi profile 'JavaSE-11' { source level: 11, target level: 11}], remote p2 repository options=org.eclipse.tycho.p2.remote.RemoteAgent@105ffc58...
[INFO] Resolving file:${project_loc:/target.refs}/base.target
[ERROR] Cannot resolve target definition:
[ERROR]   N/A
[ERROR]
[ERROR] Failed to resolve target definition file:/C:/ws/target-testcase/tycho/target.references/target.refs/target.refs.target: Invalid URI file:C:\ws\target-testcase\tycho\target.references\target.refs/base.target: Illegal character in opaque part at index 7: file:C:\ws\target-testcase\tycho\target.references\target.refs/base.target -> [Help 1]
(...)

It appears that ${project_loc} is being expanded using Windows-native backslashes, which is not acceptable when the path is going to be part of a URI. The same problem appears in PDE, and it prevents the nested target functionality from being usable in either environment, as I have not found any feasible workarounds for the issue.

My suggestion would to implement one of two options,:

• Add new ${X_uri} variables parallel to project_loc, workspace_loc, etc. that are guaranteed to expand to a valid URI, thus something like file:/C:/path/with/forward/slashes in Windows
• Rewriting the handling of relative file: URIs to ensure that they are interpreted as relative to a reasonable default e.g. the folder containing the pom.xml and/or the target file would be a good option.

Either case would have to apply also to PDE (see issue in progress) in order to have a fully compatible environment with Tycho.

Environment info:

Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
Maven home: C:\Users\jvma\devtools\apache-maven-3.6.3
Java version: 11.0.10, vendor: AdoptOpenJDK, runtime: C:\Users\jvma\devtools\jdk-11.0.10+9
Default locale: es_ES, platform encoding: Cp1252
OS name: "windows 10", version: "10.0", arch: "amd64", family: "windows"

[laeubi]

• Rewriting the handling of relative file: URIs to ensure that they are interpreted as relative to a reasonable default

See Bug 578429

My suggestion would to implement one of two options,

As \ is an invalid character anyways it might be better so simply replace all \ with / when converting the raw string to an URI.

[habbit-baggins]

As \ is an invalid character anyways it might be better so simply replace all \ with / when converting the raw string to an URI.

That seems reasonable, and much quicker than either of my options. However, the URI for a Windows path would be file:/C:/path, while your suggestion would produce file:C:/path (missing the first slash). Both seem to work in my machine both in Maven and testing things out with URIs in jshell.

See Bug 578429

Regarding this, would it mean that ${target_loc} would point to the main target platform file, or to each one? Because the former makes A→B chains usable, but the latter would be needed to have A→B→C chains, I think.

[habbit-baggins]

Oh wow, that was fast! Thanks a lot, I will try to ping the PDE side too so that hopefully both sides are compatible in Eclipse 4.23/Tycho 2.7 (or 2.6.1 if this is considered relevant enough to make a point release).

[laeubi]

${target_loc}

Target loc should always point to the absolute path of the current target file, but we first need to convince the PDE people to actually enhance the support (currently it refers to the location of the current active target location if it is a file).

So if you consider it useful or have any remarks drop a comment in the referenced bug to let other people know this is an issue for you.

[laeubi]

I will try to ping the PDE side too so that hopefully both sides are compatible in Eclipse 4.23

I'll prepare a patch for PDE if we have verified it works on Tycho.

Tycho 2.7

Tycho 2.7 will be released soon so I don't think there is much need for 2.6.1 release. If you like you can try to came up with an example and provide an integration-test to demonstrate the issue, but all tests run on linux so one might need to adjust one of the existing tests to use a "windows style" linux path.

[habbit-baggins]

all tests run on linux so one might need to adjust one of the existing tests to use a "windows style" linux path

Hmm actually \ is a valid character for a path component in Linux, so I could make an IT that has a base file named ba\se.target or something like that. However, then the path converting to / would not work. Or actually, having a path with any character that is not valid in a URI/URL. Instead of converting the slashes we could take the ${project_loc} and URL encode it on/after expansion. Doing it in Windows generates a URI like file:/D%3A%5Cpath%5Cbase.target which is ugly but does work.

But perhaps I should raise a separate issue for that, since the "working on Windows" part is indeed fixed by your patch.

[laeubi]

I will just add a unit test for the conversion of your example that might be sufficient already.

[laeubi]

Hmm actually \ is a valid character for a path component in Linux

I think this is not really a path component but a shell escape character, as far as I know java do not use the escapes internally in string representations.

[habbit-baggins]

No, I mean that a file in Linux can legitimately be called my text\data.txt. If you wanted to use it from the shell you would indeed use \ to escape both the space and the other backslash; or just wrap everything into single quotes, but the file name itself does validly contain \, it's not an escape:

$ touch 'my text\data.txt'
$ cat my\ text\\data.txt
$ ls
'my text\data.txt'  pom.xml  target.refs  test.bundle

However, as I said, this can be raised as a separate issue regarding file paths with special chars, since this patch should indeed fix my original issue. Thanks a lot!

[laeubi]

Could you try out the current tycho (3.0.0-SNAPSHOT) snapshot build if the problem is fixed there?

[laeubi]

Cherry picked to 2.7.0 branch.

[habbit-baggins]

I confirm it works both on Windows and Linux. Many thanks for the super quick turnaround!

[INFO] Resolving target definition file:/C:/ws/target-testcase/tycho/target.references/target.refs/target.refs.target for environments=[win32/win32/x86_64], include source mode=honor, execution environment=StandardEEResolutionHints [executionEnvironment=OSGi profile 'JavaSE-11' { source level: 11, target level: 11}], remote p2 repository options=org.eclipse.tycho.p2.remote.RemoteAgent@5b715ea...
[INFO] Resolving file:${project_loc:/target.refs}/base.target
[INFO] Reading target file:/C:/ws/target-testcase/tycho/target.references/target.refs/base.target...
[INFO] Performing subquery
(...)