You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/12/02 13:52:28 UTC
[GitHub] [daffodil-site] stevedlawrence opened a new pull request #74: Use the official Jekyll container to build the site
stevedlawrence opened a new pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74
Using a container makes it much easier to ensure consistency between
different developer environments, and even avoids devs needing to setup
an environment and deal with dependency issues.
- Update the README to describe how to test locally using the Jekyll
container
- Update GitHub Actions to use the official Jekyll container
- Update GitHub Actions to run on pull requests, but with the publish
step skipped, so that PRs can test that everything builds correctly
before being merged
- Commit the Gemfile.lock file to freeze dependency versions
- Remove asciidoc descriptions for blockdiag, seqdiag, actdiag, and
netdiag. The dependencies for these diagrams aren't easily added to
the Jekyll container, we don't use any of them, and there is no
obvious benefit to them over PlantUML or GraphViz
DAFFODIL-2595
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence commented on a change in pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#discussion_r761355916
##########
File path: .github/workflows/main.yml
##########
@@ -13,45 +13,32 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-name: Publish Daffodil Site
+name: CI
-on: [push]
+on:
+ push:
+ pull_request:
+ types: [opened, synchronize, reopened]
jobs:
- test:
- name: Publish Site
- runs-on: ubuntu-latest
+ build-publish:
+ name: Build & Publish
strategy:
fail-fast: false
- steps:
+ runs-on: ubuntu-20.04
+ container:
+ image: jekyll/jekyll:4.2.0
Review comment:
Opened [INFRA-22580](https://issues.apache.org/jira/browse/INFRA-22580) to see about allowing use of the Jekyll container
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence commented on a change in pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#discussion_r761345264
##########
File path: .github/workflows/main.yml
##########
@@ -13,45 +13,32 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-name: Publish Daffodil Site
+name: CI
-on: [push]
+on:
+ push:
+ pull_request:
+ types: [opened, synchronize, reopened]
jobs:
- test:
- name: Publish Site
- runs-on: ubuntu-latest
+ build-publish:
+ name: Build & Publish
strategy:
fail-fast: false
- steps:
+ runs-on: ubuntu-20.04
+ container:
+ image: jekyll/jekyll:4.2.0
Review comment:
Sigh, looks like there's a problem with this approach. Seems apache has restrictions on what actions can be run a a step, and Apache policy doesn't explicitly allow this container to be used, here's the error:
> docker://jekyll/jekyll:4.2.0 is not allowed to be used in apache/daffodil-site. Actions in this workflow must be: within a repository owned by apache, created by GitHub, verified in the GitHub Marketplace or match the following: */*@[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]+, AdoptOpenJDK/install-jdk@*, JamesIves/github-pages-deploy-action@5dc1d5a192aeb5ab5b7d5a77b7d36aea4a7f5c92, TobKed/label-when-approved-action@*, actions-cool/issues-helper@*, actions-rs/*, al-cheb/configure-pagefile-action@*, amannn/action-semantic-pull-request@*, apache/*, burrunan/gradle-cache-action@*, bytedeco/javacpp-presets/.github/actions/*, chromaui/action@*, codecov/codecov-action@*, conda-incubator/setup-miniconda@*, container-tools/kind-action@*, container-tools/microshift-action@*, dawidd6/action-download-artifact@*, docker://pandoc/core:2.9, eps1lon/actions-label-merge-conflict@*, gaurav-nelson/github-action-markdown-link-check@*, golangci/*, gr2m/twitter-together@*, gradle/wrappe
r-va...
I'm not sure how this is any different than running all actions in this container like I had before. Maybe infra can allow this container to the allowed list, seems something that other projects might want to use.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence commented on a change in pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#discussion_r761291377
##########
File path: .apk
##########
@@ -0,0 +1 @@
+graphviz
Review comment:
Unfortunately comment don't work. The container does this t install packages from this file
```bash
apk add --no-cache --no-progress $(cat .apk)
````
So any comments aren't processed as comments and are just treated as additional things to install.
We really need the container to do something like this:
```bash
apk add --no-cache --no-progress $(grep -v '^#' .apk)
```
But we can't make that change without updating the container.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence commented on pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#issuecomment-984886170
from @mbeckerle
> w.r.t. the docker/podman images like the jekyll predefined one we're depending on. (and redhat one for the main daffodil build) Do we want to make clones/forks of those to insure their stability.
The containers should always be stable and unchanging. We always depend on specific tagged version of a container, and that version shouldn't change (assuming container owners don't overwrite existing versions). Accessing `jekyll/jeyll-4.2.0` or `fedora:35` should always be the exact same container no matter what. So I don't think we need to fork or clone them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence commented on a change in pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#discussion_r761323199
##########
File path: .github/workflows/main.yml
##########
@@ -13,45 +13,32 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-name: Publish Daffodil Site
+name: CI
-on: [push]
+on:
+ push:
+ pull_request:
+ types: [opened, synchronize, reopened]
jobs:
- test:
- name: Publish Site
- runs-on: ubuntu-latest
+ build-publish:
+ name: Build & Publish
strategy:
fail-fast: false
- steps:
+ runs-on: ubuntu-20.04
+ container:
+ image: jekyll/jekyll:4.2.0
Review comment:
I had the same questions, and it took me some research and trial and error to figure out what it actually does.
Essentially GitHub action starts the container with the entrypoint as `/usr/bin/tail` before any steps are run, so it doesn't actually use the entrypoint defined by the container. And then for each step, it basically runs `docker exec ...` for whatever the step does. So all the work actually takes place inside the container, including the checkout, build, publish, etc.
I think this means the container needs to have some basic functionality for github actions to actually work in the container, but it seems the jekyll container has everything that is needed for the few things we do (checkout and publish).
Alternatively, you can run a container for just a single step, which is what I originally tried and couldn't get working, but actually think this works:
```
- name: Build
uses: docker://jekyll/jekyll:4.2.0
env:
JEKYLL_ROOTLESS: 1
with:
args: jekyll build --source site
```
That seems to mount the github workspace as volume and sets it the mounted volume as the working directory, which is all the container really needs to do it's thing. I think this is a be a bit more clear, and only uses the container for building which is probably a better approach. I want to test the publish part of this, but if it works, I think I'll switch to this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence merged pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence merged pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] tuxji commented on a change in pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
tuxji commented on a change in pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#discussion_r761273835
##########
File path: .github/workflows/main.yml
##########
@@ -13,45 +13,32 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-name: Publish Daffodil Site
+name: CI
-on: [push]
+on:
+ push:
+ pull_request:
+ types: [opened, synchronize, reopened]
jobs:
- test:
- name: Publish Site
- runs-on: ubuntu-latest
+ build-publish:
+ name: Build & Publish
strategy:
fail-fast: false
- steps:
+ runs-on: ubuntu-20.04
+ container:
+ image: jekyll/jekyll:4.2.0
+ steps:
- name: Checkout Repository
- uses: actions/checkout@v1.0.0
-
- - name: Install Ruby
- uses: actions/setup-ruby@v1
- with:
- ruby-version: '2.6'
-
- - name: Install Python
- uses: actions/setup-python@v1
+ uses: actions/checkout@v2.4.0
with:
- python-version: '3.5'
-
- - name: Install Dependencies
- run: |
- sudo apt-get install graphviz
- bundle install
- python -m pip install --upgrade pip
- pip install blockdiag
- pip install seqdiag
- pip install actdiag
- pip install nwdiag
+ fetch-depth: 0
Review comment:
How much does leaving out the git history speed up the CI? My checkout is 89M, just curious.
##########
File path: .apk
##########
@@ -0,0 +1 @@
+graphviz
Review comment:
Are you allowed to put a comment in this file? If so, I'd like a comment saying something like (correct me if my understanding is wrong):
`# Install additional system packages in Jekyll container`
##########
File path: .github/workflows/main.yml
##########
@@ -13,45 +13,32 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-name: Publish Daffodil Site
+name: CI
-on: [push]
+on:
+ push:
+ pull_request:
+ types: [opened, synchronize, reopened]
jobs:
- test:
- name: Publish Site
- runs-on: ubuntu-latest
+ build-publish:
+ name: Build & Publish
strategy:
fail-fast: false
- steps:
+ runs-on: ubuntu-20.04
+ container:
+ image: jekyll/jekyll:4.2.0
Review comment:
This is the first time I've seen a job using a container image. How is the container image able to see the checkout of the git repository? Does GitHub Actions start the container with an automatic bind mount of $GITHUB_WORKSPACE behind the scenes? How do you control where the container mounts $GITHUB_WORKSPACE in its filesystem so that the jekyll build command is able to use a relative filename like "site"?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence commented on a change in pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#discussion_r761299127
##########
File path: .github/workflows/main.yml
##########
@@ -13,45 +13,32 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-name: Publish Daffodil Site
+name: CI
-on: [push]
+on:
+ push:
+ pull_request:
+ types: [opened, synchronize, reopened]
jobs:
- test:
- name: Publish Site
- runs-on: ubuntu-latest
+ build-publish:
+ name: Build & Publish
strategy:
fail-fast: false
- steps:
+ runs-on: ubuntu-20.04
+ container:
+ image: jekyll/jekyll:4.2.0
+ steps:
- name: Checkout Repository
- uses: actions/checkout@v1.0.0
-
- - name: Install Ruby
- uses: actions/setup-ruby@v1
- with:
- ruby-version: '2.6'
-
- - name: Install Python
- uses: actions/setup-python@v1
+ uses: actions/checkout@v2.4.0
with:
- python-version: '3.5'
-
- - name: Install Dependencies
- run: |
- sudo apt-get install graphviz
- bundle install
- python -m pip install --upgrade pip
- pip install blockdiag
- pip install seqdiag
- pip install actdiag
- pip install nwdiag
+ fetch-depth: 0
Review comment:
The checkout only took 4 seconds with `fetch-depth: 0` for this PR check. I just committed a branch without fech-depth to leave out the history, and it took 6 seconds. Not sure why that's the case, maybe the checkout speed has more to do with the network? Either way, I think the answer is it doesn't make much difference.
And the full history is actually needed in order for the publish to work since that makes a new commit to the asf-site branch. Not sure if there's a way to do that with the shallow checkout.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [daffodil-site] stevedlawrence commented on a change in pull request #74: Use the official Jekyll container to build the site
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #74:
URL: https://github.com/apache/daffodil-site/pull/74#discussion_r761332755
##########
File path: .github/workflows/main.yml
##########
@@ -13,45 +13,32 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-name: Publish Daffodil Site
+name: CI
-on: [push]
+on:
+ push:
+ pull_request:
+ types: [opened, synchronize, reopened]
jobs:
- test:
- name: Publish Site
- runs-on: ubuntu-latest
+ build-publish:
+ name: Build & Publish
strategy:
fail-fast: false
- steps:
+ runs-on: ubuntu-20.04
+ container:
+ image: jekyll/jekyll:4.2.0
Review comment:
The above worked, and I think is much cleaner. Added as a fixup commit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org