Build From Specific Commit

[sonic2kk]

I'm tinkering around with using this project to build an updated Yad binary (carrying on the legacy of Frostworx with SteamTinkerLaunch 😄) and I would like to build Yad specifically from the v13.0 release (v1cont/yad@fbfb0fa).

I believe this should be possible by updating build.sh to take a commit argument. Perhaps the simplest way to do this would be something alone these lines before:

# Define GIT repo
declare -r GIT='https://github.com/v1cont/yad'
declare -r COMMIT="$1"

# ...

if [ -n "$COMMIT" ]; then
    git checkout "$COMMIT"
fi

# Build the application

This will add some "noise" output with git warning about being in a detached state, but this is probably fine I think.

I have no idea if it is this simple, I'll test it out locally and see if it works.

---

If this feature is desired, we can discuss approaches if the above is not suitable to merge, and also I am happy to get a PR up for this :-)

Thanks!

[sonic2kk]

Ah right, Docker. I'm not sure how it all works, but I guess the commit would need to be passed when building the Docker image.

For my purposes, I can just hardcode the commit, but of course for a PR we would need a more flexible solution :-)

[kadogo]

Hello @sonic2kk

I'm completely fine with your approach. It would normally work also locally, it's just that for building a specific version it can be easier to use docker.

If you can have it working, I would happily accept a pr. Or I can give it a try today in the evening if you prefer, but I think using a git checkout like you suggest is the right approach.

[kadogo]

I got a bit a time before a meeting and I got it working, and it looks working properly. I got Yad version 13 based on your commit above. I did the build on bookworm, but it could be something else.

diff --git a/yad/Dockerfile b/yad/Dockerfile
index 50822ec..7a27cb8 100644
--- a/yad/Dockerfile
+++ b/yad/Dockerfile
@@ -1,4 +1,4 @@
-FROM debian:stretch
+FROM debian:bookworm
 
 # Install basic applications
 RUN apt update
diff --git a/yad/build.sh b/yad/build.sh
index 63503c5..acc29d4 100755
--- a/yad/build.sh
+++ b/yad/build.sh
@@ -2,11 +2,17 @@
 
 # Define GIT repo
 declare -r GIT='https://github.com/v1cont/yad'
+declare -r COMMIT="fbfb0fafb06fad14b5ddc2d7a9a3064022ee41b4"
 
 # Clone the GIT
 git clone "$GIT" ./git
 cd ./git
 
+# Checkout COMMIT
+if [[ -n "$COMMIT" ]] ; then
+  git checkout "$COMMIT"
+fi
+
 # Build the application
 autoreconf -ivf
 intltoolize
diff --git a/yad/dependencies.sh b/yad/dependencies.sh
index 57b7293..169185d 100755
--- a/yad/dependencies.sh
+++ b/yad/dependencies.sh
@@ -3,12 +3,16 @@
 # Install dependencies
 # autoconf
 # automake
+# gcc
 # intltool
+# make
 # gtk3
 # Need libglib2.0-dev for an issue during autoreconf -ivf (build.sh)
 apt install --force-yes --yes \
   autoconf \
   automake \
+  gcc \
   intltool \
+  make \
   libgtk-3-dev \
   libglib2.0-dev

I don't mind the COMMIT hard-coded, to be honest. When I wrote easy-builder I was thinking like you have a specific recipe, and it must build that recipe. I'm not against another branch that is more dynamic like passing arg through docker or the build script but I don't think it's mandatory.

If the process looks good for you, I will update it, but maybe using bullseye instead of bookworm to keep a kind of "compatibility".

[sonic2kk]

Wow! Thanks for your quick work here.

The process looks great! If you're fine having a hardcoded commit for Yad then I think that's acceptable as well. It's pretty rare to need a specific commit for any other reason, and in the worst case, someone can just edit the script manually for their specific use-case until if/when a more flexible method may be needed.

I was able to build a Yad AppImage with the patch you provided, applied to a fresh clone.

I ran into a problem with ./create-appimage.sh which may be worth talking about. It seems LinuxDeploy is broken for some users, and that included me. Its calls to strip fail. You can see more here: linuxdeploy/linuxdeploy#272 - Fwiw, running the Dockerfile with the patch you provided in an endeavourOS VM ran fine, although the binary was 10mb and didn't use a system GTK theme, The small binary size makes me suspicious that the AppImage may not be as portable as one that is built outside of Docker. But I could be talking nonsense :-)

The fix suggested there is also what I had to do: Set NO_STRIP=true. I also had to do one more step, which was to create the AppImage target folder. Actually I modified the create-appimage.sh script to do mkdir -p ../target and then changed the AppImage output location to ../target. I ended up with this:

diff --git a/yad/create-appimage.sh b/yad/create-appimage.sh
index 9fcd2eb..48fca65 100755
--- a/yad/create-appimage.sh
+++ b/yad/create-appimage.sh
@@ -20,15 +20,17 @@ StartupNotify=true
 EOF
 
 # Build appimage with linuxdeploy
-../squashfs-root/AppRun \
+NO_STRIP=true ../squashfs-root/AppRun \
   --appdir ./AppDir \
   -i ./data/icons/128x128/yad.png \
   --executable ./AppDir/bin/yad \
   -d ./yad.desktop \
   --output appimage
 
+
 # Move AppImage to /target
-mv ./*.AppImage /target/
+mkdir -p ../target/
+mv ./*.AppImage ../target/
 
 # Execute the CMD command by default in Dockerfile or what we pass as argument
 exec "$@"

This may conflict with using Docker, perhaps simply mkdir -p is sufficient. Although a log message noting where target is at the end of the script might be handy, basically saying echo "Successfully created AppImage and it can be found at $(realpath target)/$( ls | head -n1 )" (bit hacky since it assumes only one appimage there but eh, hope you get the idea anyway :-))

But messing with the path was just my preference. The only thing I really had to do was set NO_STRIP=true. Maybe there could be an option exposed for this when running ./create-appimage.sh?

Not stripping the libraries did inflate the binary size by about 19mb (79mb up from 60mb), but I'm willing to accept this for the SteamTinkerLaunch use-case. This problem is also not on your side as far as I understand, it's a problem with LinuxDeploy. I just wanted to bring it up that I ran into this general obstacle. Even if you don't think it's worth it to include such an option, at least it is documented here in this issue :-)

---

tl;dr Using a fixed Yad commit is totally fine with me, but there are upstream issues with LinuxDeploy that I had to workaround to build the AppImage. Perhaps incorporating a change like this to create-appimage.sh in alongside the changes to build.sh is worth considering, as well as some logging for where to find the AppImage (unless such a message already exists and I missed it because I haven't had enough coffee 😉)

Thanks again for your quick reply!

[kadogo]

Hello @sonic2kk

On which distribution you got the error with linuxdeploy?
I didn't need the NO_STRIP, but I'm using Debian 12 and I used the same docker image, so maybe it's related to the distribution?

I don't mind adding an option at the start of the script, like an optional option that can be enabled or not.

The target can, perhaps, be more customizable. With docker, I just link the $(pwd)/target/ to /target, but an option in the script is maybe a good idea too. I don't know if you think that would be fine?

Have a good day and welcome ^^

(I must be a bit sleepy to use the wrong window ^^)

[sonic2kk]

On which distribution you got the error with linuxdeploy?

I use Arch btw 😉 Just a vanilla Arch install, no special flavour. The error didn't come up in the Docker container so perhaps it works fine on Debian (or other Debian-based distros).

The linked LinuxDeploy issue is from another user on Arch, and online the issues that eventually led me to finding the LinuxDeploy issue, those were also from people using Arch (tauri-apps/tauri#8929) except for one user I saw on Fedora 39 (ninia/jep/#446).

I think this is further upstream than this project though, I don't think this is a problem created on your end. 🙂 Ideally it would be patched upstream, although maybe the option to disable the strip would be useful to some people (can't really say I have a preference either way).

The target can, perhaps, be more customizable. With docker, I just link the $(pwd)/target/ to /target, but an option in the script is maybe a good idea too. I don't know if you think that would be fine?

Linking in this way sounds fine to me :-)

[kadogo]

I think it's more safe to have the flag available then, Debian is always a bit late, but it could happen pretty fast.

I will give it a try with the flag enable, if it's working I think it's fine to let it available by default. I will still read a bit about what it is doing just in case ^^

I will try to have something update for the weekend or maybe before but it doesn't sound too much complicate to do.

[sonic2kk]

I'm happy to test anything here as well, I'm always browsing around GitHub for one reason or another so I'm usually not too hard to get a hold of :-)

[kadogo]

Hello @sonic2kk

I give a try on Debian, and it's still working, so I kept the NO_STRIP option

Can you give a try with the branch https://github.com/kadogo/easy-builder/tree/add-more-customization

If it's good for you, I will merge it like that and keep an issue open to update the other software one day if I need it or if someone wants to do some PR ^^

[sonic2kk]

Just gave the branch a try and it all worked, except even with a target folder in ./git/target, the create-appimage.sh script was complaining about not being able to find the folder. Removing the / at the beginning of /target/ in the script fixed the issue:

diff --git a/yad/create-appimage.sh b/yad/create-appimage.sh
index 9842bbe..54c5b09 100755
--- a/yad/create-appimage.sh
+++ b/yad/create-appimage.sh
@@ -6,7 +6,7 @@
 export NO_STRIP=1
 
 # TARGET path
-declare -r TARGET="/target/"
+declare -r TARGET="target/"
 
 # Extract linuxdeploy se we not use fuse for building
 ./linuxdeploy.AppImage --appimage-extract

I fully realise the target variable is intentionally there to make it easier to customise the output location, it's just that removing the / at the beginning should hopefully allow a "default" to work. On that, perhaps a `mkdir -p "$TARGET" could be useful, to create the path if it doesn't exist. But I get it if you'd rather not get into the realm of handholding with which paths do and don't exist, especially when we're dealing with relative paths here :-)

Apart from that literal one-character change, this worked out of the box for me on Arch! The binary is bigger now that NO_STRIP is being used, but that's by design since the libraries aren't being stripped, and the increase is probably not going to be super huge. This is also an upstream issue, not an issue with your project.

Thanks for the super fast work on this!

[kadogo]

Hello @sonic2kk

Sorry, I had completely forgotten this issue. I tried without the "/" but it failed, so instead I added an if that check the variable container when we run the docker with the docker run command.

I think it would work properly, but please let me know if it doesn't.

It could be interesting to try to adapt that to all the other applications.

[kadogo]

You can use the branch https://github.com/kadogo/easy-builder/tree/add-more-customization