-
Notifications
You must be signed in to change notification settings - Fork 153
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
Fix parsing classpath for Bumblebee, Chipmunk and Dolphin on Linux and Windows #442
Conversation
c0f2ec1
to
47f496c
Compare
@@ -41,16 +41,19 @@ some other text | |||
studioInstallDir.toPath().resolve("lib/lib2.jar"), | |||
studioInstallDir.toPath().resolve("lib/lib1/lib1.jar") | |||
] | |||
|
|||
where: | |||
classpathKeyword << ["CLASS_PATH", "CLASSPATH"] |
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.
Let's document somewhere which keyword belongs to the old vs. new versions.
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.
Good idea, added a comment.
* Android Studio Dolphin (2021.3) and older versions use "CLASSPATH", while Electric Eel (2022.1) and newer versions use CLASS_PATH | ||
*/ | ||
private static final Pattern LINUX_CLASSPATH_LIB_PATTERN = Pattern.compile(".*(CLASS_PATH|CLASSPATH)=.*(?<lib>lib/.+\\.jar).*"); | ||
private static final Pattern WINDOWS_CLASSPATH_LIB_PATTERN = Pattern.compile(".*(CLASS_PATH|CLASSPATH)=.*(?<lib>lib\\\\.+\\.jar).*"); |
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.
Nit: instead of (CLASS_PATH|CLASSPATH)
you could use (?:CLASS_PATH|CLASSPATH)
to make it obvious that this group is not captured.
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.
Fixed
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.
LGTM.
If this will be still require adjustments with new versions in the future, we could run
start.sh
orstart.bat
directly, but we would need to check what env. variables we need to set for custom AS args.We don't have AS multi version tests, but I ran a tests with Bumblebee and all pass, here is the build:
https://builds.gradle.org/buildConfiguration/GradleProfiler_GradleProfilerTestTrigger/55190392?buildTab=overview&hideProblemsFromDependencies=false&hideTestsFromDependencies=false&expandBuildChangesSection=true