Fix ubuntu 22 #41
Fix ubuntu 22 #41
Conversation
This should make this action usable on any debian-based distros.
src/main.ts
Outdated
| const files: Array<string> = output.split('\n').filter((f: string) => { | ||
| try { | ||
| return fs.statSync(f).isFile() | ||
| } catch (e) { | ||
| return false | ||
| } | ||
| }) |
There was a problem hiding this comment.
Am I understand right that here we catch an exception from fs.statSync() regarding the non-existent file? If so, it would be more accurate to pass the throwIfNoEntry: false flag.
However, silent ignoring of all the non-existent files anyway looks a bit dangerous. I guess I can propose a better way.
See, the root of the problem is in dpkg excludes in the docker container.
$ cat /etc/dpkg/dpkg.cfg.d/excludes
# Drop all man pages
path-exclude=/usr/share/man/*
# Drop all translations
path-exclude=/usr/share/locale/*/LC_MESSAGES/*.mo
# Drop all documentation ...
path-exclude=/usr/share/doc/*
# ... except copyright files ...
path-include=/usr/share/doc/*/copyright
# ... and Debian changelogs for native & non-native packages
path-include=/usr/share/doc/*/changelog.*apt-get install cmake respects them, but dpkg -L doesn't. The file is created during build of the docker image, see here.
AFAIS, dpkg -L doesn't respect to explicitly passed --exclude-path and --include-path options. But we can just implement this filtering in Typescript.
I would hardcode the exclude/include rules as in the file listed above in the action code.
I don't insist, here are just my thoughts around.
However, I would describe the reason of the problem a bit deeper in the commit message (like it is done in this message, for example).
There was a problem hiding this comment.
I would also add that it is more related to run in a docker container job rather than Ubuntu of a particular version. I guess that it is as related to Ubuntu 22.04 as for 20.04.
| @@ -197,7 +209,7 @@ async function run_linux(): Promise<void> { | |||
|
|
|||
| await core.group('Setting up repository', async () => { | |||
| await exec.exec('sudo tee /etc/apt/sources.list.d/tarantool.list', [], { | |||
| input: Buffer.from(`deb ${baseUrl}/ubuntu/ ${distro} main\n`) | |||
| input: Buffer.from(`deb ${baseUrl}/${distro_id}/ ${distro} main\n`) | |||
There was a problem hiding this comment.
I vote up this enhancement, but it doesn't look related to the pull request title: it is not about Ubuntu 22.04.
Since the pull request is more about docker container jobs rather than specific to Ubuntu 22.04 I would rephrase the pull request title/description.
|
@grafin, thanks for the patchset! The changes are useful, look visually correct and AFAIK are tested on a private project. However, there are some details I would polish before push. First, pull request's title/description is a bit misleading: it seems, the changes are about docker container jobs in general rather than about specific Ubuntu 22.04. It would be nice to clarify. Next, since our goal here is to make container jobs working, I would add some testing with |
In ubuntu:22.04 docker man dpkg contains followng: "-L|--listfiles <package>... List files 'owned' by package(s)." dpkg -L cmake mentions file /usr/share/doc/cmake/NEWS.Debian.gz, which is not present in the system after successful apt-get -y install cmake. This led to setup-tarantool failing during packaging cache. This patch catches error while calling io.cp and prints it as a warning.
grafin
force-pushed
the
master
branch
2 times, most recently
from
e3ac99a to
dbb505a
Compare
|
Closed in favor of #45 (new PR needed because I created a branch here to test new test:). I fixed all these review comments there. |
|
@grafin Thanks! I'll take a look on a spare time. |
There where several errors, which prevented this action to be used in ubuntu:22.04 docker container. See commits for details.