Skip to content

Allow install scripts to be used by koreader CI#71

Merged
yparitcher merged 2 commits into
koreader:masterfrom
yparitcher:master
Jan 2, 2022
Merged

Allow install scripts to be used by koreader CI#71
yparitcher merged 2 commits into
koreader:masterfrom
yparitcher:master

Conversation

@yparitcher

@yparitcher yparitcher commented Dec 27, 2021

Copy link
Copy Markdown
Member

This is part of work to also use Github actions as CI

Use distro luarocks
Install packages globally
Use latest busted
Dont install luacov-coveralls & luasec: we don't use them at all
There is no need to explicitly install luarocks dependencies


This change is Reviewable

@yparitcher

Copy link
Copy Markdown
Member Author

Please review

Comment thread docker/ubuntu/baseimage/install_lint.sh Outdated

@Frenzie Frenzie left a comment

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.

lgtm

Use latest busted

dont install luacov-coveralls & luasec: we don't use them at all

There is no need to explicitly install dependencies
@yparitcher yparitcher merged commit a64fd88 into koreader:master Jan 2, 2022
@yparitcher

Copy link
Copy Markdown
Member Author

Closes: #70

@yparitcher yparitcher mentioned this pull request Jan 2, 2022
Frenzie added a commit to koreader/koreader that referenced this pull request Jan 2, 2022
@Frenzie

Frenzie commented Jan 2, 2022

Copy link
Copy Markdown
Member

Btw, I just realized luasec was probably in there because the sync server came first.

https://github.com/koreader/koreader-sync-server/blob/d7d1ebff54240cfbade96a81f2971b6ad0afa33f/Dockerfile#L27-L32


Cross-ref to koreader/koreader#8607

#!/usr/bin/env bash

HOME=/home/ko
apt-get install --no-install-recommends luarocks

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.

@yparitcher This cannot have ever worked, can it?

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.

No, it cannot.

If we want to use distro luarocks, it should be done in https://github.com/koreader/virdevenv/blob/11b62a5722b3adc1d98ce8484b89968b136db4c5/docker/ubuntu/baseimage/bootstrap.sh really

But you could do it here with sudo in theory, see

Keep in mind we purposefully remove the apt cache lines earlier

rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*

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.

2 participants