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

Implement get_executable_path() for macOS #5560

Merged
merged 6 commits into from
Dec 2, 2024
Merged

Implement get_executable_path() for macOS #5560

merged 6 commits into from
Dec 2, 2024

Conversation

rom1v
Copy link
Collaborator

@rom1v rom1v commented Nov 28, 2024

Implement the function to get the executable path on macOS, so that enabling -Dportable=true works on all platforms to find scrcpy-server and icon.png in the same directory as the executable.

@Genxster1998 I stole this from your branch. Thank you 😉

Please review/test on macOS.

@Genxster1998
Copy link
Contributor

Genxster1998 commented Nov 29, 2024

@rom1v I tested the build from exec_path branch, it works as expected with both wrapper and directly executing scrcpy_bin (with default adb installation) in macOS 14.5 x86_64. i would suggest to make changes for bundled adb path too.

@rom1v
Copy link
Collaborator Author

rom1v commented Nov 30, 2024

One more thing, the scrcpy script does not work if called from a symlink.

For example:

$ ln -s scrcpy-linux-x86_64-v3.0/scrcpy aaa
$ ./aaa
./aaa: line 7: ./scrcpy_bin: No such file or directory

On Linux, readlink -f solves the problem:

diff --git app/data/scrcpy_static_wrapper.sh app/data/scrcpy_static_wrapper.sh
index b31b4f1f4..9e765ca87 100755
--- app/data/scrcpy_static_wrapper.sh
+++ app/data/scrcpy_static_wrapper.sh
@@ -1,4 +1,4 @@
 #!/bin/bash
-cd "$(dirname ${BASH_SOURCE[0]})"
+cd "$(dirname $(readlink -f ${BASH_SOURCE[0]}))"
 export ADB="${ADB:-./adb}"
 ./scrcpy_bin "$@"

also in the case where it's a relative symlink pointing to a relative symlink (which does not work without -f).

However, on macOS, I think readlink behaves differently (-f is not available). There is realpath, but it is not installed by default.

@Genxster1998
Copy link
Contributor

Genxster1998 commented Nov 30, 2024

https://stackoverflow.com/questions/7665/how-to-resolve-symbolic-links-in-a-shell-script
Will check few fixes from above.
Yeah ,a wrapper posix shell script won't be stable as binary itself, it would have problem resolving links,args,params etc.
Edit: readlink without -f option works in macOS.
For me realpath is installed at /bin/realpath signed by apple on macOS Sonoma. on some prev. versions some claim its not available.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 1, 2024

I added commits so that scrcpy uses the local adb in portable builds, and I removed the wrapper script completely.

Please test/review 😉

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 1, 2024

Edit: readlink without -f option works in macOS.

Without -f, it does not if there are two levels of relative symlinks (on Linux at least).

For example:

ln -s scrcpy-linux-x86_64-v3.0/scrcpy aaa
mkdir bbb
cd bbb
ln -s ../aaa ccc
./ccc

@metayan
Copy link

metayan commented Dec 1, 2024

Trying to build 39c3921 on macOS 10.15.7 fails with

[63/130] Compiling C object app/scrcpy.p/src_sys_unix_file.c.o
FAILED: app/scrcpy.p/src_sys_unix_file.c.o 
cc -Iapp/scrcpy.p -Iapp -I../app -I../app/src -I/usr/local/include -I/usr/local/include/SDL2\
 -I/usr/local/Cellar/libusb/1.0.27/include/libusb-1.0 -fdiagnostics-color=always\
 -Wall -Winvalid-pch -Wextra -std=c11 -O0 -g -Wmissing-prototypes -D_THREAD_SAFE -MD -MQ\
 app/scrcpy.p/src_sys_unix_file.c.o -MF app/scrcpy.p/src_sys_unix_file.c.o.d\
 -o app/scrcpy.p/src_sys_unix_file.c.o -c ../app/src/sys/unix/file.c
../app/src/sys/unix/file.c:66:9: error: call to undeclared function '_NSGetExecutablePath';\
 ISO C99 and later do not support implicit function declarations\
 [-Wimplicit-function-declaration]
   66 |     if (_NSGetExecutablePath(buf, &bufsize) != 0) {
      |         ^
1 error generated.

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 2, 2024

@metayan Does this help:

diff --git app/src/sys/unix/file.c app/src/sys/unix/file.c
index 60c806bb2..6123c788c 100644
--- app/src/sys/unix/file.c
+++ app/src/sys/unix/file.c
@@ -6,6 +6,9 @@
 #include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#ifdef __APPLE__
+# include <mach-o/dyld.h> // for _NSGetExecutablePath()
+#endif
 
 #include "util/log.h"
 

?

@Genxster1998
Copy link
Contributor

Genxster1998 commented Dec 2, 2024

@rom1v
Yeah ,that was obvious, It won't resolve all symbolic links. I linked the stackoverflow query discussing the same.

@metayan
Copy link

metayan commented Dec 2, 2024

@rom1v
The include solved it. Builds and works fine.

By the way, the release scripts are also excellent, taking care of the dependencies.
Very useful.

Noticed this, though, unrelated to this PR, but appears when building on macOS:

ld: warning: could not create compact unwind for _kbd_window_init:
 stack subq instruction is too different from dwarf stack size

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 2, 2024

ld: warning: could not create compact unwind for _kbd_window_init:
 stack subq instruction is too different from dwarf stack size

https://stackoverflow.com/questions/39598302/ld-warning-stack-subl-instruction-is-too-different-from-dwarf-stack-size-on-os

🤷

Genxster1998 and others added 6 commits December 2, 2024 18:23
Contrary to getenv(), sc_get_env() returns an allocated string that is
guaranteed to be encoded in UTF-8 on all platforms (it uses _wgetenv()
internally on Windows and converts the strings).

PR #5560 <#5560>
Contrary to getenv(), the result of sc_get_env() is encoded in UTF-8 on
all platforms. Since it is allocated, it requires an explicit init() and
destroy() functions.

PR #5560 <#5560>
For non-Windows portable builds, use the absolute path to the adb
executable located in the same directory as scrcpy.

On Windows, just use "adb", which is sufficient to use the local one.

PR #5560 <#5560>
Log the ADB executable path (at the DEBUG level) if it is not the
default one.

PR #5560 <#5560>
All portable builds now use the files located in the same directory as
the scrcpy executable by default.

PR #5560 <#5560>
@rom1v rom1v merged commit aea6a37 into dev Dec 2, 2024
@rom1v
Copy link
Collaborator Author

rom1v commented Dec 2, 2024

Merged into dev.

I will probably publish a release 3.0.1 soon with the current dev branch, which fixes several issues from 3.0.

@Genxster1998
Copy link
Contributor

Genxster1998 commented Dec 3, 2024

I added commits so that scrcpy uses the local adb in portable builds, and I removed the wrapper script completely.

You mean it checks for system wide adb env var or not before using local adb?

@rom1v
Copy link
Collaborator Author

rom1v commented Dec 3, 2024

By default, it uses the adb the local directory.

This can be changzd by setting the ADB envvar.

For example:

ADB=adb scrcpy

It will use adb (like in non-portable builds), which will be found in PATH.

@Genxster1998
Copy link
Contributor

Genxster1998 commented Dec 3, 2024

@metayan I would strongly suggest you to move to later macOS releases preferably latest 2 releases.Catalina ceased to get security updates in oct 2022 vulnerable to easy root exploits like dirtycow patched in dec 2022 or other tcc exploits.

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.

3 participants