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

fix: fix CleanUp classpath bug #4416

Closed
wants to merge 2 commits into from
Closed

Conversation

sam80180
Copy link

no need to fix compiled artifact's name to '/data/local/tmp/scrcpy-server.jar'

no need to fix compiled artifact's name to '/data/local/tmp/scrcpy-server.jar'
@rom1v
Copy link
Collaborator

rom1v commented Nov 11, 2023

This is an idea, but the classpath will not necessary contains only the scrcpy-server jar to cleanup. For example, we might add other jars in the classpath: #3927 (comment)

@sam80180
Copy link
Author

I tried to use DexClassLoader to solve the problem.

rom1v added a commit that referenced this pull request Nov 21, 2023
The path can be retrieved from the classpath.

PR #4416 <#4416>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v
Copy link
Collaborator

rom1v commented Nov 21, 2023

Thank you. I took your commit and simplified (by making assumptions): e76ff25 (on PR #4446, it simplified a lot my work because now the scrcpy-server jar name is random on the device).

@sam80180
Copy link
Author

sam80180 commented Nov 22, 2023

if scrcpy is not the first item in CLASSPATH, you code just doesn't work. Better not make such assumption, as CLASSPATH order is up to users.

@rom1v
Copy link
Collaborator

rom1v commented Nov 22, 2023

scrcpy is not the first item in CLASSPATH, you code just doesn't work.

There are only two cases:

Therefore, running more complex code handling any order, with relative or absolute paths, opening jar files and iterating over all the classes (internally) is unnecessary: more runtime execution (probably negligible) and more risk of bugs (on any custom ROM). If the server path cannot be determined, that would make scrcpy fail.

Better not make such assumption, as CLASSPATH order is up to users.

The server is intended to work with the scrcpy client, it is not a generic library. Although there are some features which facilitate its use without the scrcpy client, its design is not constrained by other use cases.

https://github.com/Genymobile/scrcpy/blob/master/doc/develop.md#standalone-server

That's why hardcoding the path was ok until I added randomness to the path name.

@rom1v
Copy link
Collaborator

rom1v commented Nov 23, 2023

I merged this commit into dev: acb2988. If we need more complex detection later, we can still modify it.

rom1v added a commit that referenced this pull request Nov 23, 2023
The path can be retrieved from the classpath.

PR #4416 <#4416>

Co-authored-by: Romain Vimont <rom@rom1v.com>
Signed-off-by: Romain Vimont <rom@rom1v.com>
@rom1v rom1v closed this Dec 1, 2023
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 this pull request may close these issues.

None yet

2 participants