Skip to content

Docker fixes - #38

Merged
ttl-octave merged 3 commits into
gnu-octave:mainfrom
pganguli:misc-fixes
Jul 27, 2026
Merged

Docker fixes#38
ttl-octave merged 3 commits into
gnu-octave:mainfrom
pganguli:misc-fixes

Conversation

@pganguli

Copy link
Copy Markdown
Contributor

Fixes for #36 (comment)

pganguli added 2 commits July 27, 2026 09:24
Running as an arbitrary host UID/GID has no matching /etc/passwd entry
inside the container, so $HOME resolves to "/" and pkg install fails
trying to create /.local/share/octave. Point HOME at the bind-mounted,
host-owned workspace instead.
GIT_CONFIG_COUNT/GIT_CONFIG_KEY_n/GIT_CONFIG_VALUE_n (added in git 2.31)
and `git -c safe.directory=...` are both silently ignored by the git
2.25.1 bundled with the gnuoctave/octave:6.4.0 and :7.3.0 images, so
the previous workaround left `git archive` failing with "detected
dubious ownership" whenever the repo isn't owned by the invoking user
(e.g. GitHub Actions container jobs, or root in a bind-mounted Docker
container). Write safe.directory to an actual (scoped, throwaway)
global gitconfig instead, which every git version here honors.
AC_CONFIG_MACRO_DIRS (plural) requires autoconf >= 2.70; the codebase
doesn't otherwise need anything from 2.70/2.71, so switch to the
singular AC_CONFIG_MACRO_DIR, which both old and new autoconf accept,
and drop AC_PREREQ accordingly. This lets `make check-ci`/bootstrap
succeed on the gnuoctave/octave:7.3.0 image, which ships Ubuntu 20.04's
autoconf 2.69. Verified bootstrap+configure+check-ci pass on octave
7.3.0, 8.4.0, 9.4.0, 10.3.0, and 11.3.0 images.
@pganguli

Copy link
Copy Markdown
Contributor Author

Yikes, it seems that v7.3.0 was still failing, but now due to an unrelated issue caused by an outdated configure.ac:

> Run make check-ci
Creating CI package dist directory target/control-4.2.2+-ci ...
  git archive ...
  copy slicot files ...
  bootstrap ...
configure.ac:4: error: Autoconf version 2.71 or higher is required
configure.ac:4: the top level
autom4te: /usr/bin/m4 failed with exit status: 63
aclocal: error: echo failed with exit status: 63
configure.ac:4: error: Autoconf version 2.71 or higher is required
configure.ac:4: the top level
autom4te: /usr/bin/m4 failed with exit status: 63
configure.ac:4: error: Autoconf version 2.71 or higher is required
configure.ac:4: the top level
autom4te: /usr/bin/m4 failed with exit status: 63
autoheader: '/usr/bin/autom4te' failed with exit status: 63
make: *** [Makefile:154: target/control-4.2.2+-ci] Error 63
Error: Process completed with exit code 2.

Pushed a fix for that too...

Comment thread devel/compose.yaml
Comment on lines +6 to +7
environment:
HOME: /workspace

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works and makes sense, but it's also worth considering using /tmp/. tmp is permissive so the .local/share (which gets created when you start octave) and .config/octave/ (which gets created when you pkg install) get created in an ephemeral /tmp/ and I don't think you'd need to add anything to the .gitignore.

I'm not 100% sure of any trade-offs but it's worth considering I think.

Comment thread Makefile

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I was trying to be clever here and use this whole GIT_CONFIG_COUNT mess to get around the fact that the tarball builds with git archive isn't run as the same user that owns the git repo (b/c it's run in docker) (after a deep rabbit hole that's thanks to CVE-2022-24765).

But it turns out that feature was added in git 2.31, and guess what versions 6.4.0 and 7.3.0 got built with:

~/workspace/pkg-control >  docker run --rm gnuoctave/octave:11.3.0 git --version
git version 2.53.0
~/workspace/pkg-control >  docker run --rm gnuoctave/octave:7.3.0 git --version
git version 2.25.1
~/workspace/pkg-control >  docker run --rm gnuoctave/octave:6.4.0 git --version
git version 2.25.1

I think your solution is probably cleaner/simpler though I'm surprised it works with safe.directory b/c that got added later? But I'm guessing it was back-ported to previous releases specifically for the CVE. At any rate, if it works it works.

@ttl-octave

Copy link
Copy Markdown
Member

Thanks for the patch, th review and the discussion. I am going to merge the pull request and will make some tests with /tmp as heom directory or at least with one single dir wihtin /workspace.

@ttl-octave
ttl-octave merged commit d1d61c8 into gnu-octave:main Jul 27, 2026
5 checks passed
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