unity-jar-resolver icon indicating copy to clipboard operation
unity-jar-resolver copied to clipboard

[Bug] Invalid path to local repository when patching mainTemplate.gradle

Open strawlink opened this issue 4 years ago • 8 comments

[REQUIRED] Please fill in the following fields:

  • Unity editor version: 2019.4.21f1
  • External Dependency Manager version: 1.2.164
  • Source you installed EDM4U: Unity Package Manager
  • Features in External Dependency Manager in use: Android Resolver, iOS Resolver
  • Plugins SDK in use: Firebase, AdMob, Facebook Audience Network, AppsFlyer, FairBid
  • Platform you are using the Unity editor on: Windows

[REQUIRED] Please describe the issue here:

(Please list the full steps to reproduce the issue. Include device logs, Unity logs, and stack traces if available.)

After updating Unity from 2019.4.9f1 to 2019.4.21f1, we needed to update our custom build pipeline: We used to use BuildOptions.AcceptExternalModificationsToPlayer in order to make Unity export a project. This was no longer a valid option, so we switched over to setting EditorUserBuildSettings.exportAsGoogleAndroidProject to true.

This has in turn caused the External Dependency Manager to patch the mainTemplate.gradle file with the absolute path to the generated local repository; this path uses an incorrect directory separator and will cause a gradle build failure.

See the backslash before "Assets":

        maven {
-            url (unityProjectPath + "/Assets/GeneratedLocalRepo/Firebase/m2repository")
+            url "file:///C:/Data/_Git/ExternalDependencyResolverIssue_InvalidPath\Assets/GeneratedLocalRepo/Firebase/m2repository"
        }

Building while the template is in this state produces the following gradle error:

FAILURE: Build failed with an exception.

* Where:
Build file 'C:\Data\_Git\ExternalDependencyResolverIssue_InvalidPath\AndroidExport\unityLibrary\build.gradle' line: 11

* What went wrong:
Could not compile build file 'C:\Data\_Git\ExternalDependencyResolverIssue_InvalidPath\AndroidExport\unityLibrary\build.gradle'.
> startup failed:
  build file 'C:\Data\_Git\ExternalDependencyResolverIssue_InvalidPath\AndroidExport\unityLibrary\build.gradle': 11: unexpected char: '\' @ line 11, column 82.
     dencyResolverIssue_InvalidPath\Assets/Ge
                                   ^

  1 error

Please answer the following, if applicable:

What's the issue repro rate? (eg 100%, 1/5 etc) 100%

What happened? How can we make the problem occur? This could be a description, log/console output, etc.

Make sure the project contains a mainTemplate.gradle file, and that External Dependency Manager is set up to patch this file. I've attached a project with a highly simplified setup: ExternalDependencyResolverIssue_InvalidPath.zip (Unity 2019.4.21f1)

  1. In File > Build Settings > Android, enable "Export Project"
  2. Save the project.
  3. Observe that the path in Plugins\Android\mainTemplate.gradle is incorrectly patched.

I've exported an Android build in the AndroidExport folder in the zip. Run gradlew build and you will see it fail as described above.

If the "Export Project" option is disabled, mainTemplate.gradle is patched back to a valid state.

Note: I would highly prefer that the fix for this issue is to not change the value to an absolute path, as it is a pain to work with in source control. This has also been mentioned in issue #239

strawlink avatar Mar 09 '21 15:03 strawlink

This issue does not seem to follow the issue template. Make sure you provide all the required information.

google-oss-bot avatar Mar 09 '21 15:03 google-oss-bot

Hi @strawlink, Thanks for providing all the details and the example project. The path not being combined properly is definitely a bug and should be easy to fix but converting to a relative path will require more thoughts. Since the exported Android project can be relocated anywhere, the only way to guarantee things are found in the right place is to provide absolute paths. If you have any other ideas or want to make a PR for this, please feel free to do so. I will add this to our bug list.

vimanyu avatar Mar 11 '21 20:03 vimanyu

Here is the code dealing with relative vs absolute paths https://github.com/googlesamples/unity-jar-resolver/blob/3dfc84aba31425c1def8a2da7027757d4b3d59fd/source/AndroidResolver/src/PlayServicesResolver.cs#L2143

vimanyu avatar Mar 11 '21 20:03 vimanyu

@vimanyu Appreciate the quick response.

I am not entirely convinced by the "exported Android project can be relocated anywhere" argument. The Unity project can also be relocated anywhere, so resolving the absolute path is not a guarantee that it would continue to work. In my opinion it would actually make it more prone to failure; is it a common practice to move an exported Android project separately from the Unity project?

I do have 2 suggestions:

  1. Adding a user-option for whether to convert the path to an absolute path. In our use-case the project is exported as part of our build pipeline, and the export location is always the same relative path to the Unity project. Adding an option would be a very simple change with no impact on existing projects.
  2. When 'Export Project' is enabled, copy the repository to be next to the exported project as a build postprocess step, and then use the relative path. This would require a bit more attention to detail and edge-case scenarios.

I'd be up for making a PR for option 1, if you agree on the proposal.

strawlink avatar Mar 17 '21 11:03 strawlink

Hi @strawlink, Option 1 sounds great. Just to preserve existing behavior, let us default the option to absolute paths. Thank you for making this contribution.

vimanyu avatar Mar 17 '21 19:03 vimanyu

+1 We ran into this unexpected char: '\' issue on our Windows build boxes last week after upgrading Firebase. Our current workaround is to string replace the character in our own post-build steps. It would be great if this could get fixed in a future release of the resolver.

StuartWebster avatar Apr 19 '21 21:04 StuartWebster

Hello. The same problem here. I found https://github.com/googlesamples/unity-jar-resolver/issues/361 and it is closed and it seems to be the same problem. I tried v1.2.165, but it gives the same result.

romeon0 avatar May 04 '21 16:05 romeon0

Hey @vimanyu Any chance the PR can be reviewed? :)

strawlink avatar Jun 29 '21 11:06 strawlink

I do have 2 suggestions:

1. Adding a user-option for whether to convert the path to an absolute path. In our use-case the project is exported as part of our build pipeline, and the export location is always the same relative path to the Unity project. Adding an option would be a very simple change with no impact on existing projects.

I completely agree with this statement. Imposing the use of absolute paths adds multiple constraints on build machines setup which are totally outside of what a project-related plugin should be allowed to impose.

Our Continuous Integration system simply are not compatible with absolute paths by design, that makes them more flexible and adaptable. Switching to absolute paths, which moreover would be specific to the developer running the Android Resolver is a recipe for additional complexity and rigidity.

The Resolver needs to be designed around the notion of a project root path. This does not seem like a particularly challenging requirement.

laurentg-nvizzio avatar Dec 12 '22 21:12 laurentg-nvizzio

I believe this should be patched awhile ago. Please open a new ticket if this still occurs on Windows machine

chkuang-g avatar Apr 14 '23 17:04 chkuang-g