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

Update Build Images #1155

Open
DanRStevens opened this issue Apr 28, 2024 · 6 comments
Open

Update Build Images #1155

DanRStevens opened this issue Apr 28, 2024 · 6 comments

Comments

@DanRStevens
Copy link
Collaborator

DanRStevens commented Apr 28, 2024

The build images haven't been updated in awhile and should be updated.


The next LTS (Long Term Support) version of Ubuntu is out this month.
https://releases.ubuntu.com/noble/

  • 24.04 (Noble Numbat)

Additionally, the MacOS build image is now quite dated.
https://circleci.com/docs/using-macos/#supported-xcode-versions

  • Most recent documented: 15.3.0 (marked as unsupported by Visual Studio code)
  • Most recent in file format schema: 15.2.0 (accepted without error underline squiggles)
  • Currently using: 13.4.1
@DanRStevens
Copy link
Collaborator Author

DanRStevens commented Aug 17, 2024

PR #1166 Added a workflow to build Docker images and push them to the GitHub Docker registry. Though it may trigger runs more often then needed.

For instance, if a Docker image build is done on a branch, and then tested by building the code using the new Docker image, and then that branch is merged to main, it will trigger another build of the Docker image. We should probably avoid that since it could potentially produce a different result from what the branch built (if some software update became available during the meantime), and so future builds would run with the different build that wasn't the one tested and verified.

It might make sense to not automatically run the new workflow for the main branch, since there is an assumption that Docker image updates will be tested and verified on a branch, and then future builds using the updated version would use the version built while testing and developing on the branch.

Potentially we could add a push branch filter of !main, and a comment explaining why.


Another weakness is that if we were to fill out the matrix to do builds for other Linux distributions, they would all build together, even if only one of them had an updated Dockerfile or version number. The limitation here is that the workflow as a whole can be triggered using a path filter, but jobs are not given such an option. If they were, we could have filtered on paths such as dockerfile/nas2d-${{ matrix.platform }}.*. Sadly, such as path filter can only be used on the workflow, and so needs to apply to all Dockerfiles.

Given the limitations of path filters, we can't prevent a matrix job from running, though we could have a job terminate early, such as by detecting if a Docker image has already been built.

The docker command line tool has a new experimental feature which can determine if an image and tag already exist in a remote repository, reporting the result using the exit code from the command. Details:

docker manifest inspect $IMGNAME:$IMGTAG > /dev/null ; echo $?

One downside to the above approach, is that while working on a branch to test and verify a new build, it's expected the Docker image will be overwritten with each new build. We don't expect the version number to be continually bumped for experimental builds. That means for potentially many builds, the image and tag will already exist, yet we still want to rebuild and update it. It's only when the branch is merged that things should be finalised.

The ability to overwrite Docker images applies to a Dockerfile that is being updated. This is separate from the problem of not updating Docker images for other Dockerfiles that are unaffected by a branch. The initial problem of this section was to avoid rebuilds of images that already exist and are not being updated on a branch.

@DanRStevens
Copy link
Collaborator Author

Further researching into detecting which files are updated on a branch, the following Stack Overflow questions are of interest:

Additionally, the following GitHub Actions documentation for push events is relevant:

It seems the previous commit can be determined with ${{ github.event.before }}. Then a git diff --name-only can be used to build a list of changed files since the last push. This can be used to determine which Docker images need to be rebuilt.

The documentation wasn't super clear about the value of github.event.before in the case of a forced push. Is it the head of the branch before the forced push, or is it the last built commit that's still on the branch after the forced push. I'm guessing probably the former. If the former, it's not clear the commit would be available in a freshly cloned repo, since it's history would effectively be erased. It might still be in the source repo, assuming it hasn't been garbage collected, but the runner instance will only be working with a clone.

@DanRStevens
Copy link
Collaborator Author

DanRStevens commented Sep 5, 2024

Running git diff with the --exit-code flag will return 1 if there were differences and 0 if there were no differences. This would be even easier than listing just the filenames of changed files and scanning them for specific files.

Using --exit-code, it may also make sense to replace --name-only with --no-patch/-s instead, which suppresses output. If all we need is the return code, then there is no need to print the filenames of changed files. Though perhaps it may still be useful for debugging or inspection purposes to print filenames of changed files.

StackOverflow: Check if specific file in git repository has changed

Example usages:

  • git diff --exit-code --name-only main -- docker/nas2d-arch.*; echo $?
  • git diff --exit-code --no-patch main -- docker/nas2d-arch.*; echo $?

@DanRStevens
Copy link
Collaborator Author

A potential problem with running git diff on a runner, is the default checkout mode uses fetch-depth: 1, so there won't be any other commits to compare with. Additionally, turning on the --depth parameter to a git fetch also enables --single-branch by default, so there won't be any information about other branches.

To add information for a single branch, such as branch main from origin:

git remote set-branches --add origin main

The commit for that branch would then need to be fetched, perhaps also using a fetch-depth of 1:

git fetch --depth 1

At that point, a comparison should then be possible:

git diff --name-only origin/main -- docker/

Information on the state of the repo (throughout the process above) can be seen with:

git remote show origin

Additionally, fetch information can be seen in .git/config.

The commands and environment can be tested locally by cloning a repo using a --depth 1 setting:

git clone --depth 1 file:///path/to/repo/nas2d-core/ clone-test

Alternatively, instead of adding a remote tracking branch, the branch can be downloaded into a local branch. Doing so is about the same for a CI environment, though perhaps less convenient long term if working locally, where updates may need to be fetched at some point in the future.

Fetching commit directly into a local branch:

git fetch --depth 1 origin main:main

Since the branch is local, the git diff needs to be slightly different (no longer using origin/main, but just main):

git diff --name-only main -- docker/

@DanRStevens
Copy link
Collaborator Author

The Mingw Docker environment shows warnings when installing the apt repo from wine.

Warning: apt-key is deprecated. Manage keyring files in trusted.gpg.d instead (see apt-key(8)).

W: https://dl.winehq.org/wine-builds/ubuntu/dists/noble/InRelease: Key is stored in legacy trusted.gpg keyring (/etc/apt/trusted.gpg), see the DEPRECATION section in apt-key(8) for details.

This stems from the use of apt-key in apt-key add -, which is now a deprecated tool.


A lot of examples on the internet give an alternative using gpg --dearmor, however, they often given instructions to place keys files in the same place that apt-key used to place them, which may be part of why it was deprecated.

Some additional info about why not to place keys in the often suggested location:

In summary, avoid using /etc/apt/trusted.gpg.d/ when installing keys, and instead use a different folder, such as /etc/apt/keyrings/.


Some additional info can be found in:

  • man sources.list
    • section signed-by
      • /usr/share/keyrings/ (for keyrings managed by packages)
      • /etc/apt/keyrings/ (for keyrings managed by the system operator)

The recommended locations for keyrings are /usr/share/keyrings for keyrings managed by packages, and /etc/apt/keyrings for keyrings managed by the system operator.

@DanRStevens
Copy link
Collaborator Author

Worked on updating the Mingw build image, though currently it's unable to run the unit test executable:

wine: failed to open "../.build/Debug_Linux_test/test": c0000135

Explicitly specifying the wine64 executable seems to allow it to run:
cd test && /usr/lib/wine/wine64 ../.build/Debug_Linux_test/test.exe

Oddly, when running the image as root to debug the issue, using just wine appeared to run the unit test executable correctly, but the same command as a more limited user would fail. Only explicitly choosing the wine64 binary would work as a normal user.


The wine command is picked up as a series of symlinks:
/usr/bin/wine -> /etc/alternatives/wine -> /usr/bin/wine-stable

It appears wine-stable is a shell script, which selects between the 32-bit or the 64-bit version, which are found at:

  • wine32=/usr/lib/wine/wine
  • wine64=/usr/lib/wine/wine64

From the script, it appears the 32-bit version is preferred, when it is available. When it's not available, a message is printed, suggesting it be installed, with instructions on how to install the 32-bit version.

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

No branches or pull requests

1 participant