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

Add GitHub Actions workflow for release builds #5306

Merged
merged 5 commits into from
Oct 27, 2024
Merged

Add GitHub Actions workflow for release builds #5306

merged 5 commits into from
Oct 27, 2024

Conversation

rom1v
Copy link
Collaborator

@rom1v rom1v commented Sep 18, 2024

Better late than never (#4490), I finally worked on a GitHub Actions workflow to build scrcpy releases.

I had to refactor the release.mk makefile to parallelize server and client build jobs. It is only run manually to build a release (not on push or pull request, I don't need it for now).

ga

In the end, it produces a zip artifact containing:

  • scrcpy-server-XXX
  • scrcpy-win32-XXX.zip
  • scrcpy-win64-XXX.zip
  • SHA256SUMS.txt

where XXX is the input name you give when you manually start the workflow (if empty, it uses the ref name, for example the branch or tag name).

To test it:

  • fork the repo to your own account
  • checkout the branch from this PR, and push it to your master branch
  • enable GitHub Actions in your repo settings
  • go to the Actions tab
  • select "Build"
  • select "Run workflow"
  • keep master branch, choose a version name (for example v42)
  • click on Run workflow

workflow


For now, it generates the same targets as ./release.sh did locally (scrcpy-server, release win32, release win64).

The goal now is to add more jobs, especially:

Your help is welcome! ❤️

Now that scrcpy-server is generated separately once and for all, it will be easier to write the jobs for building the scrcpy client for different targets (Android SDK is not needed).

As an important constraint, the jobs MUST only reference "official" github actions (or dockers images), not actions/images from random users. Keep it as simple as possible.


Fixes #4490
Refs #2256 (comment) and next comments (cc @dur-randir @LeeBinder @Coool)
Refs #1709 (cc @dylanmtaylor)
Refs #3721 (cc @qmfrederik)
Refs #4427 (cc @hacksysteam)
Refs #4489 (cc @Gary-Cod)

@LeeBinder
Copy link

Awesome, great work, Romain. I'm recovering health-wise right now which might take a while so will be mostly offline for the near future :/

@dur-randir
Copy link

dur-randir commented Sep 20, 2024

I've added macos build job + modified makefile in my fork at dur-randir@6c4c2c3. The next question is, this generates unsigned binaries - e.g. you either have to provide a signing key (see https://docs.github.com/en/actions/use-cases-and-examples/deploying/installing-an-apple-certificate-on-macos-runners-for-xcode-development) or a user have to perform additional steps after download (like brew does automatically when installing from casks).

@Genxster1998
Copy link

I've added macos build job + modified makefile in my fork at dur-randir@6c4c2c3. The next question is, this generates unsigned binaries - e.g. you either have to provide a signing key (see https://docs.github.com/en/actions/use-cases-and-examples/deploying/installing-an-apple-certificate-on-macos-runners-for-xcode-development) or a user have to perform additional steps after download (like brew does automatically when installing from casks).

amd64 aka x86_64 build actions config ? I can't see them there.

@rom1v
Copy link
Collaborator Author

rom1v commented Sep 23, 2024

@dur-randir Thank you very much!

A few questions/remarks:

  • Could app/deps/ffmpeg-macos.sh be "merged" into app/defs/ffmpeg.sh to avoid duplication (a common file for all platforms)?
  • It seems only FFmpeg is built for macOS, not the other dependencies (SDL and libusb)? (and an adb binary for macOS)
  • Is macpack absolutely necessary? Can we build a release without this tool?

I'll read about signing keys later.

@dur-randir
Copy link

dur-randir commented Sep 23, 2024

Could app/deps/ffmpeg-macos.sh be "merged" into app/defs/ffmpeg.sh to avoid duplication (a common file for all platforms)?

Sure, I've done it separately because of I didn't know in advance what'd be needed to be changed. There's dievergence just in two places - additional args list for configure disable-debug..enable-videotoolbox and export PATH, which can be put under if $HOST in deps/common.

It seems only FFmpeg is built for macOS, not the other dependencies (SDL and libusb)? (and an adb binary for macOS)

They're taken from brew install in binary form.

Is macpack absolutely necessary? Can we build a release without this tool?

Yes and no. I've tried (again!) to make a fully static build (link ffmpeg + libusb + libsdl) but I don't know meson at all and it broke somewhere down the road while double-linking _sha_$whatever while I was trying to shove them all together and then I ceased trying. Without static linking with those dependencies, one must relocate all libraries to be searched by relative paths by DYLD (they're baked by absolute paths by default on macos). IIRC this can be done with native instruments, but they're non-recursive, so I've picked one tool that works recursively out-of-the box. Another one tool is https://github.com/auriamg/macdylibbundler which can be installed from brew. Or create this recursive hand-written loop for path change. Or ideally make a fully static build, but I've given up. If you know meson, maybe you can create a recipe for at least linux, so I can tweak it for macos?

@rom1v
Copy link
Collaborator Author

rom1v commented Sep 23, 2024

Sure, I've done it separately because of I didn't know in advance what'd be needed to be changed.

👍

They're taken from brew install in binary form.

OK 👍 In the end, I think we want to build them without depending on brew.

IIRC this can be done with native instruments, but they're non-recursive, so I've picked one tool that works recursively out-of-the box.

Indeed, the native tool seems to be install_name_tool (which is used by macpack).

For windows, the scrcpy builds have 6 dll (so these libs are linked dynamically):

  • avcodec-61.dll
  • avformat-61.dll
  • avutil-59.dll
  • swresample-5.dll
  • libusb-1.0.dll
  • SDL2.dll

I think the macOS build could do the same, there would be no need for finding dependencies recursively.

@Genxster1998
Copy link

Genxster1998 commented Oct 9, 2024

Here, i added for MacOS x86-64 thanks to @dur-randir db01900
artifacts

One can use adhoc-sign for SIP disabled Systems, idk about others. Need to put scrcpy-server separately in same dir.

@LeeBinder
Copy link

LeeBinder commented Oct 9, 2024

Awesome, you ROCK, @Genxster1998 - big THANK you 👍 !

These two script files come in handy 😉:

assign icon.png to scrcpy binary.zip

scrcpy-launcher.zip

  1. Unpack them into the same dir as scrcpy, scrcpy-server etc.
  2. run 'assign icon.png to scrcpy binary' first (only 1x to assign an icon so it appears in the dock rather than the black terminal icon)
  3. run 'scrcpy-launcher' to launch scrcpy

@LeeBinder
Copy link

.. and for those who like it as a native macOS app 😀:

scrcpy 2.7 (Intel) app.zip

@Genxster1998
Copy link

@LeeBinder , you won't need a launcher to furnish path to scrcpy-server as it has been set default to executable_path already rather than /opt/... so no need envt. just adb,scrcpy,libs,server in same dir after patching binary for icon. i think *.dylib(s), adb and scrcpy-server can be moved to Frameworks in proper macOS app.

run 'scrcpy-launcher' to launch scrcpy

@LeeBinder
Copy link

LeeBinder commented Oct 10, 2024

@Genxster1998 ideally, but it's not working (yet). Because when I simply double-click the scrcpy binary even if scrcpy-server IS present in the same directory, I get:

ERROR: Could not get local file path, using scrcpy-server from current directory
stat: No such file or directory
ERROR: 'scrcpy-server' does not exist or is not a regular file
ERROR: Server connection failed

Try it yourself, please.

Looks as if you need to add something in the compile/ build script so that scrcpy indeed DOES look in the same directory. Something is still missing.

@rom1v
Copy link
Collaborator Author

rom1v commented Oct 10, 2024

Looks as if you need to add something in the compile/ build script so that scrcpy indeed DOES look in the same directory.

The option is portable:

option('portable', type: 'boolean', value: false, description: 'Use scrcpy-server from the same directory as the scrcpy executable')

(add -Dportable=true to the meson command)

However, the code to find the executable path is not implemented for macos:

// in practice, we only need this feature for portable builds, only used on
// Windows, so we don't care implementing it for every platform
// (it's useful to have a working version on Linux for debugging though)
return NULL;

@Genxster1998
Copy link

Genxster1998 commented Oct 10, 2024

@LeeBinder Thats true indeed , Launching app from launchpad or Finder is not working. I tried adding LSEnvironment dict keys in info.plist which is specifically made for same purpose. yet it still fails to acknowledge server path from there. btw it works with open -a scrcpy.app in terminal. @rom1v Yeah, it is compiling with portable boolean true for meson, i guess a static path could be assigned for MacOS GUI app in ../Resources or ../Libraries Directory. Anyway , a real gui mac app should have a config window/menu/ to assign params for stream size, window size etc or select device.

@LeeBinder
Copy link

I see. Agreed, should eventually be in ../Resources (neither in ../Libraries nor in ../Frameworks because it's neither of those but a Android executable binary)

The prepare-deps recipe does not exist anymore. It has been split into
prepare-deps-win32 and prepare-deps-win64.

PR #5306 <#5306>
This will allow to run server tests separately on the CI.

PR #5306 <#5306>
This avoids to install meson/ninja to build scrcpy-server on the CI.

PR #5306 <#5306>
Make it possible to build scrcpy-server and Windows binaries in
parallel from different GitHub Actions workflows, and to package
everything as a final step.

PR #5306 <#5306>
@rom1v
Copy link
Collaborator Author

rom1v commented Oct 27, 2024

The scripts to build dependencies are currently specific to cross-compile Windows from Linux. I'll have to adapt them to also support native build (will be useful for macOS). Meanwhile, I can merge this PR.

@rom1v rom1v merged commit a5844e1 into dev Oct 27, 2024
rom1v added a commit to rom1v/scrcpy that referenced this pull request Nov 22, 2024
Since commit 2687d20, the Makefile
named release.mk stopped handling dependencies between recipes, because
they have to be executed separately (from different Github Actions
jobs).

There is no real benefit using a Makefile anymore. Replace them by
several individual release scripts for simplicity and readability.

Refs Genymobile#5306 <Genymobile#5306>
@rom1v
Copy link
Collaborator Author

rom1v commented Nov 22, 2024

See #5515.

rom1v added a commit that referenced this pull request Nov 24, 2024
Since commit 2687d20, the Makefile
named release.mk stopped handling dependencies between recipes, because
they have to be executed separately (from different Github Actions
jobs).

Using a Makefile no longer provides any real benefit. Replace it by
several individual release scripts for simplicity and readability.

Refs #5306 <#5306>
PR #5515 <#5515>
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.

4 participants