Add native docker images w/ build workflow #316

joeyparrish · 2024-03-10T00:36:13Z

Rebased, cleaned-up version of PR #292, which adds Docker images that do not rely on Wine, with a GitHub Actions workflow to build and push Docker images to ghcr.io.

Compared with #292, this:

is based on 4397ac2 (2024-03-06) instead of d84d456 (2023-07-31)
has no changes to newline styles, so the diff is easier to read
adds instructions to readme.md on building deps/gcc.Dockerfile
fixes case on gcc docker image (must be lowercase apparently)
fixes typos
streamlines and simplifies the GitHub Actions workflow
disables arm64 image builds because they keep hanging in the GitHub Actions environment

Resolves #290
Resolves #304

joeyparrish · 2024-03-11T17:00:14Z

I'm debugging and streamlining the workflows on my fork. I'll convert this to a draft for now until that process is complete. Sorry for the noise!

joeyparrish · 2024-03-11T21:42:52Z

This is ready now. Please take a look!

Rebased, cleaned-up version of PR Stephane-D#292, which adds Docker images that do not rely on Wine, with a GitHub Actions workflow to build and push Docker images to ghcr.io. Compared with Stephane-D#292, this: - is based on 4397ac2 (2024-03-06) instead of d84d456 (2023-07-31) - has no changes to newline styless, so the diff is easier to read - adds instructions to readme.md on building deps/gcc.Dockerfile - fix case on gcc docker image (must be lowercase apparently) - fixes typos - streamlines and simplifies the GitHub Actions workflow - disables arm64 image builds because they keep hanging in the GitHub Actions environment Resolves Stephane-D#290 Resolves Stephane-D#304

loki666 · 2024-03-12T08:07:08Z

Nice, could you add git to the second stage image.
It would be handy when using the image in a VS Code devcontainer.

Thanks

Stephane-D · 2024-03-12T21:32:10Z

makefile.gen

 SRC_S= $(wildcard *.s)
 SRC_S+= $(wildcard $(SRC)/*.s)
 SRC_S+= $(wildcard $(SRC)/*/*.s)
 SRC_S+= $(wildcard $(SRC)/*/*/*.s)
-SRC_S:= $(filter-out $(SRC)/boot/sega.s,$(SRC_S))


Why it was removed ? this line is important so we can add a specific rule to build boot/sega.s file

I'm reverting this. See also my other replies.

Stephane-D

See comments in Dockerfile and mekafile.gen

Stephane-D · 2024-03-12T21:43:34Z

makefile.gen

 	$(CC) $(DEFAULT_FLAGS) -c $< -o $@

-$(SRC)/boot/sega.s: $(SRC_LIB)/boot/sega.s
+$(OUT)/boot/sega.s: $(SRC_LIB)/boot/sega.s


Why did you moved boot/sega.s to OUT folder ? The file make be customized (interrupt callback, bootstrap..) so i think it's important to keep it in SRC folder

I inherited most of these changes from the original PR. I just tried to clean it up for review and fix the workflows.

I'm guessing the original author thought that generated files should go to OUT as a rule, and didn't realize that it could be customized. I would have made the same mistake.

I'll revert this.

I have run into one problem with these generated sources, though, which I will try to resolve. If I rebuild SGDK, my existing src/boot/rom_head.c gets overwritten when I rebuild my ROM because the one in SRC_LIB is newer. I only noticed because I had already customized my rom_head.c file.

Reverted sega.s moving to OUT. Fixed overwritten boot files.

Stephane-D · 2024-03-12T21:48:57Z

Dockerfile

+WORKDIR $SGDK_PATH/tools/apj/src
+RUN javac sgdk/aplib/*.java
+RUN jar cfe $SGDK_PATH/bin/apj.jar sgdk.aplib.Launcher sgdk/aplib/*.class
+
+# Building lz4w.jar
+WORKDIR $SGDK_PATH/tools/lz4w/src
+RUN javac sgdk/lz4w/*.java
+RUN jar cfe $SGDK_PATH/bin/lz4w.jar sgdk.lz4w.Launcher sgdk/lz4w/*.class
+
+# Building sizebnd.jar
+WORKDIR $SGDK_PATH/tools/sizebnd/src
+RUN javac sgdk/sizebnd/*.java
+RUN jar cfe $SGDK_PATH/bin/sizebnd.jar sgdk.sizebnd.Launcher sgdk/sizebnd/*.class
+
+# Building rescomp.jar
+WORKDIR $SGDK_PATH/tools/rescomp/src
+ENV CLASSPATH="$SGDK_PATH/bin/apj.jar:$SGDK_PATH/bin/lz4w.jar:$SGDK_PATH/tools/rescomp/src"
+RUN cp -r $SGDK_PATH/tools/commons/src/sgdk .
+RUN find . -name "*.java"
+RUN find . -name "*.java" | xargs javac
+RUN echo -e "Main-Class: sgdk.rescomp.Launcher\nClass-Path: apj.jar lz4w.jar" > Manifest.txt
+RUN jar cfm $SGDK_PATH/bin/rescomp.jar Manifest.txt  .


Is there any interest in rebuilding the JAR file here instead of just copying them from the SGDK repo ? JAR should be portable.

Sure, I'll look into it.

This already seems to rebuild the jar from scratch. I deleted the jar from my local repo, reran the Docker build, and everything is fine.

With this Dockerfile, I can delete all .exe, .dll, and .jar files from the repo and be perfectly happy in Linux. Everything built from source, no Wine. Deleting those from the repo would resolve #297, but I'm not sure if that would have negative consequences for Windows users. Should I do it here? In a follow-up PR? Not at all?

Thanks!

joeyparrish · 2024-03-14T22:48:47Z

Nice, could you add git to the second stage image. It would be handy when using the image in a VS Code devcontainer.

I would like to keep this PR focused. When it is merged, and we have build pipelines that automatically publish new Docker images, you could always follow up with a PR to add git. And ultimately, it will be @Stephane-D who decides whether or not to allow that.

joeyparrish · 2024-03-14T22:49:50Z

makefile.gen


-out/sega.o: $(SRC)/boot/sega.s out/rom_head.bin
-	$(CC) -x assembler-with-cpp -Wa,--register-prefix-optional,--bitwise-or $(AFLAGS) -c $(SRC)/boot/sega.s -o $@
+$(OUT)/sega.o: $(OUT)/boot/sega.s $(OUT)/rom_head.bin


This doesn't actually seem to depend on rom_head.bin, so I'm removing that dependency.

I stand corrected. sega.s references rom_head.bin. I didn't realize. I'll add a comment.

joeyparrish · 2024-03-14T22:50:45Z

makefile.gen


 clean: cleanobj cleanres cleanlst cleandep
-	$(RM) -f out.lst out/cmd_ out/symbol.txt out/rom.nm out/rom.wch out/rom.bin
+	$(RM) -f out.lst $(OUT)/cmd_ $(OUT)/symbol.txt $(OUT)/rom.nm $(OUT)/rom.wch $(OUT)/rom.bin


I don't know what out.lst is exactly, but it looks like it belongs in $(OUT), so I'm moving it.

joeyparrish · 2024-03-14T23:02:21Z

Dockerfile

-ENV GDK=/sgdk
-ENV SGDK_DOCKER=y
+WORKDIR /sgdk
+COPY . .


I'm reorganizing the steps so more of them can be cached. When I change any one file in SGDK and rebuild, everything past this point must be rerun because the source folder changed and the cache of this step and those after it become stale.

joeyparrish · 2024-03-14T23:29:09Z

@Stephane-D, I believe I have addressed all the feedback so far. My only outstanding question for you is this:

This Dockerfile can regenerate all binaries. I can delete all .exe, .dll, .jar, and .a files from the repo and everything still works from source natively on Linux. Deleting those from the repo would resolve #297, but I'm not sure if that would have negative consequences for Windows users. Should I do it here? In a follow-up PR? Not at all?

Thanks!

Stephane-D · 2024-03-16T00:10:19Z

@Stephane-D, I believe I have addressed all the feedback so far. My only outstanding question for you is this:

This Dockerfile can regenerate all binaries. I can delete all .exe, .dll, .jar, and .a files from the repo and everything still works from source natively on Linux. Deleting those from the repo would resolve #297, but I'm not sure if that would have negative consequences for Windows users. Should I do it here? In a follow-up PR? Not at all?

Thanks!

@joeyparrish Thanks for taking the time to fix all that !
Well about the JAR file rebuild, it's just that i'm afraid that this increase significantly the Docker call. As I understand it, it will rebuild all tools at each time will compile through the Docker call right ?

joeyparrish · 2024-03-16T03:33:43Z

Well about the JAR file rebuild, it's just that i'm afraid that this increase significantly the Docker call. As I understand it, it will rebuild all tools at each time will compile through the Docker call right ?

Those steps (building tools, jars, etc) will only run when someone runs "docker build ..." to create the image from SGDK source. The GitHub Actions workflow will build images every time you push to the master branch. These are all the steps with "RUN".

When a user wants to compile their ROM with SGDK using Docker, the tools will not be rebuilt. It will start at "ENTRYPOINT".

Does this answer your concern?

joeyparrish · 2024-03-22T00:57:56Z

@Stephane-D, let me know if you have other concerns or if you would like me to make other changes. I believe the PR is in a good state. Everything is rebuilt from source at Docker build time in the automated workflow. Each change to the master branch will trigger a new Docker image with native Linux binaries to be published via GitHub. And the Docker image is now based on Alpine Linux instead of Ubuntu, which should make it much smaller and faster to download.

iratahack · 2024-03-22T03:02:34Z

@joeyparrish , any thoughts on using crosstool-ng and bulding the latest gcc 13.2.0?

Also, Sjasm sources are included under tools as a submodule. There is no need to clone independently.

joeyparrish · 2024-03-22T16:47:32Z

@joeyparrish , any thoughts on using crosstool-ng and bulding the latest gcc 13.2.0?

Also, Sjasm sources are included under tools as a submodule. There is no need to clone independently.

Looks like it would be useful. I could explore redoing the GCC Dockerfile with crosstool-ng after this PR is merged. Or you could send a PR yourself if you already have experience with crosstool-ng. (I have none.)

iratahack · 2024-03-22T18:21:50Z

@joeyparrish , any thoughts on using crosstool-ng and bulding the latest gcc 13.2.0?
Also, Sjasm sources are included under tools as a submodule. There is no need to clone independently.

Looks like it would be useful. I could explore redoing the GCC Dockerfile with crosstool-ng after this PR is merged. Or you could send a PR yourself if you already have experience with crosstool-ng. (I have none.)

I have the Dockerfile specific to building the tools using crosstool-ng but I may need some assistance integrating it. Maybe a collaboration? Take a look at the Dockerfile here iratahack@ba9fc47

joeyparrish · 2024-03-27T15:47:48Z

@iratahack, I was hoping to get this PR merged as a baseline before I moved on to improvements, so I would at least be able to refer to the official Docker image from ghcr.io for my own project. But since this hasn't been re-reviewed, I'll take a look at your work and try to integrate it if I can. It may be days to weeks before I have the time, though. Thank you for your offer of collaboration!

joeyparrish mentioned this pull request Mar 10, 2024

issues in Docker #304

Closed

joeyparrish marked this pull request as draft March 11, 2024 17:00

joeyparrish changed the title ~~Add multi-platform docker image~~ Add native docker images w/ build workflow Mar 11, 2024

joeyparrish force-pushed the docker branch 2 times, most recently from d2c638f to 328de48 Compare March 11, 2024 21:39

joeyparrish marked this pull request as ready for review March 11, 2024 21:40

joeyparrish force-pushed the docker branch from 328de48 to ec7dd55 Compare March 11, 2024 22:56

Stephane-D reviewed Mar 12, 2024

View reviewed changes

Stephane-D requested changes Mar 12, 2024

View reviewed changes

joeyparrish commented Mar 14, 2024

View reviewed changes

joeyparrish added 3 commits March 14, 2024 16:26

Reorg Dockerfile for improved caching

fce0dfd

Address feedback

437dff5

Fix overwriting of boot sources

22d0f22

joeyparrish requested a review from Stephane-D March 22, 2024 00:53

Stephane-D merged commit c6cb89b into Stephane-D:master Mar 29, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add native docker images w/ build workflow #316

Add native docker images w/ build workflow #316

joeyparrish commented Mar 10, 2024 •

edited

Loading

joeyparrish commented Mar 11, 2024

joeyparrish commented Mar 11, 2024

loki666 commented Mar 12, 2024

Stephane-D Mar 12, 2024 •

edited

Loading

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

Stephane-D left a comment

Stephane-D Mar 12, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

Stephane-D Mar 12, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish commented Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish Mar 14, 2024

joeyparrish commented Mar 14, 2024

Stephane-D commented Mar 16, 2024 •

edited

Loading

joeyparrish commented Mar 16, 2024

joeyparrish commented Mar 22, 2024

iratahack commented Mar 22, 2024 •

edited

Loading

joeyparrish commented Mar 22, 2024

iratahack commented Mar 22, 2024

joeyparrish commented Mar 27, 2024

Add native docker images w/ build workflow #316

Add native docker images w/ build workflow #316

Conversation

joeyparrish commented Mar 10, 2024 • edited Loading

joeyparrish commented Mar 11, 2024

joeyparrish commented Mar 11, 2024

loki666 commented Mar 12, 2024

Stephane-D Mar 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stephane-D left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeyparrish commented Mar 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeyparrish commented Mar 14, 2024

Stephane-D commented Mar 16, 2024 • edited Loading

joeyparrish commented Mar 16, 2024

joeyparrish commented Mar 22, 2024

iratahack commented Mar 22, 2024 • edited Loading

joeyparrish commented Mar 22, 2024

iratahack commented Mar 22, 2024

joeyparrish commented Mar 27, 2024

joeyparrish commented Mar 10, 2024 •

edited

Loading

Stephane-D Mar 12, 2024 •

edited

Loading

Stephane-D commented Mar 16, 2024 •

edited

Loading

iratahack commented Mar 22, 2024 •

edited

Loading