Skip to content

[WIP] Dockerless#8428

Closed
yparitcher wants to merge 1 commit into
koreader:masterfrom
yparitcher:dockerless
Closed

[WIP] Dockerless#8428
yparitcher wants to merge 1 commit into
koreader:masterfrom
yparitcher:dockerless

Conversation

@yparitcher

@yparitcher yparitcher commented Nov 8, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Superseded by #8607

This change is Reviewable

@Frenzie

Frenzie commented Nov 8, 2021

Copy link
Copy Markdown
Member

Well, that's annoying.

@yparitcher

yparitcher commented Nov 8, 2021

Copy link
Copy Markdown
Member Author

Well, that's annoying.

Why?
I am just trying to test if this makes sense, and the only way to get the action to run is to open a PR.

It actually runs the same speed if not faster without docker.

@Frenzie

Frenzie commented Nov 8, 2021

Copy link
Copy Markdown
Member

Why?

It's annoying that it works. Nothing you did! ;-)

It actually runs the same speed if not faster without docker.

It had better. ^_^

@yparitcher

yparitcher commented Nov 8, 2021

Copy link
Copy Markdown
Member Author

There are some tests/checks that are only on the .ci/ scripts, i will have to add them to kodev to be sure.
also this allows the test to be more similar to the dev environment (both using ./kodev) and hopefully less fragile. (if it breaks online it should break locally)

@yparitcher

yparitcher commented Nov 8, 2021

Copy link
Copy Markdown
Member Author

The GHA ubuntu images comes with a bunch of preinstalled software (and i suspect they have an APT cache) so instalation takes ~30 sec which is the same time it would take to download the docker image

@yparitcher

yparitcher commented Nov 8, 2021

Copy link
Copy Markdown
Member Author

https://github.com/actions/virtual-environments/blob/main/images/linux/Ubuntu2004-README.md

@Frenzie

Frenzie commented Nov 8, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Why?

Also it's very annoying because I very much want to get rid of maintaining various duplicate things, which takes a lot of extra wasted effort.

@Frenzie

Frenzie commented Nov 8, 2021

Copy link
Copy Markdown
Member

There are some tests/checks that are only on the .ci/ scripts, i will have to add them to kodev to be sure.

For sure? I thought it was the other way around.

@Frenzie

Frenzie commented Nov 8, 2021

Copy link
Copy Markdown
Member

My bad, it's identical. Where did you think you saw a difference?

koreader/kodev

Line 980 in 3dabbd5

--exclude-tags=notest "${test_path}"

koreader/Makefile

Line 143 in 3dabbd5

--exclude-tags=notest $(BUSTED_OVERRIDES) $(BUSTED_SPEC_FILE)

@yparitcher

yparitcher commented Nov 8, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

The unscaled sizes checks, shfmt, shellcheck

Frenzie
Frenzie reviewed Nov 8, 2021
View reviewed changes
Comment thread .github/workflows/tests.yml Outdated
run: sudo apt-get update && sudo apt-get install -y luarocks gettext ccache libsdl2-2.0-0

- name: Shfmt
run: sudo curl -sSL "https://github.com/mvdan/sh/releases/download/v3.4.0/shfmt_v3.4.0_linux_amd64" -o "/usr/local/bin/shfmt" && sudo chmod +x "/usr/local/bin/shfmt"

@Frenzie Frenzie Nov 8, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And there's the reason for:

koreader/.ci/install.sh

Lines 51 to 69 in 3dabbd5

#install our own updated shellcheck
SHELLCHECK_VERSION="v0.7.1"
SHELLCHECK_URL="https://github.com/koalaman/shellcheck/releases/download/${SHELLCHECK_VERSION?}/shellcheck-${SHELLCHECK_VERSION?}.linux.x86_64.tar.xz"
if ! command -v shellcheck; then
curl -sSL "${SHELLCHECK_URL}" | tar --exclude 'SHA256SUMS' --strip-components=1 -C "${HOME}/bin" -xJf -
chmod +x "${HOME}/bin/shellcheck"
shellcheck --version
else
echo -e "${ANSI_GREEN}Using cached shellcheck."
fi
# install shfmt
SHFMT_URL="https://github.com/mvdan/sh/releases/download/v3.2.0/shfmt_v3.2.0_linux_amd64"
if [ "$(shfmt --version)" != "v3.2.0" ]; then
curl -sSL "${SHFMT_URL}" -o "${HOME}/bin/shfmt"
chmod +x "${HOME}/bin/shfmt"
else
echo -e "${ANSI_GREEN}Using cached shfmt."
fi

Which is annoyingly duplicate with the Docker image in virdevenv. :-)

(Incidentally, it'd surprise me if the GHA shellcheck is quite up to date? And if it is, will it break without warning when it's updated?)

@yparitcher yparitcher Nov 8, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

shellcheck 0.7.0-2build2

@Frenzie Frenzie Nov 8, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, that's old. We want 0.7.2. (Well, actually we want 0.8.0 which just came out going forward but it might catch some new stuff. :-) )

I hadn't updated the shellcheck in .ci/install.sh with the express purpose of switching over to 0.7.2 in the Docker image.

@Frenzie

Frenzie commented Nov 8, 2021

Copy link
Copy Markdown
Member

The unscaled sizes checks, shfmt, shellcheck

Oh, but those have to run before doing anything else, not after.

@yparitcher

yparitcher commented Nov 8, 2021

Copy link
Copy Markdown
Member Author

The unscaled sizes checks, shfmt, shellcheck

Oh, but those have to run before doing anything else, not after.

I am running the checks first

@yparitcher yparitcher mentioned this pull request Dec 26, 2021
Closed
3 tasks
@yparitcher yparitcher closed this Jan 2, 2022
@yparitcher yparitcher deleted the dockerless branch January 2, 2022 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Frenzie Frenzie Frenzie left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yparitcher @Frenzie