Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switching from Madoko to AsciiDoc for P4Runtime specification #510

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 36 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
4f8de37
Add reference for RFC2697 in bib
Dscano Aug 20, 2024
adcfbe9
Merge branch 'p4lang:main' into main
Dscano Oct 13, 2024
7659239
Merge branch 'p4lang:main' into main
Dscano Nov 11, 2024
780d780
Switching from Madoko to AsciiDoc for P4Runtime specification
Dscano Nov 16, 2024
c43d938
Addressed some comments in the PR
Dscano Nov 17, 2024
3d45a90
Addressed some comments in the PR, part 2
Dscano Nov 19, 2024
207dd87
Addressed some comments in the PR, part 3
Dscano Nov 19, 2024
f6703ed
Addressed some comments in the PR, part 4
Dscano Nov 21, 2024
ef538d6
Addressed some comments in the PR, part 5
Dscano Nov 23, 2024
4b9875f
Addressed some comments in the PR, part 6
Dscano Nov 23, 2024
dbbfe09
Addressed some comments in the PR, part 7
Dscano Nov 23, 2024
faf7275
Addressed some comments in the PR, part 8
Dscano Nov 24, 2024
4c48e1b
Removed trailing whitespace. Add lint script
Dscano Nov 24, 2024
4e277de
Changed caption table 7
Dscano Nov 24, 2024
36c2602
Fix tables caption
Dscano Nov 24, 2024
8f0f754
Proposal to address LibreOffice comment
Dscano Dec 2, 2024
7fbb83d
Implemented proposal (a) to update image and generate via LibreOffice
Dscano Dec 2, 2024
50b8bf5
Remove rogue version from Dockerfile.asciidoc
Dscano Dec 2, 2024
9eae1e4
Fix Dockerfile.asciidoc
Dscano Dec 2, 2024
6a58e48
README updated, and PNG, SVG images removed from the repo
Dscano Dec 7, 2024
d56cb14
Merge branch 'p4lang:main' into main
Dscano Dec 13, 2024
3d6caa4
Merge branch 'p4lang:main' into main
Dscano Dec 22, 2024
c0e0239
fix workflows yml
Dscano Dec 23, 2024
25118c6
fix Dockerfile.asciidoc
Dscano Dec 24, 2024
71bc292
Fix the Dockerfile.asciidoc by adding the time. Correct typos in READ…
Dscano Dec 24, 2024
c2355be
Address issue 528, used correct markup for em-dashes
Dscano Dec 24, 2024
02f0f21
Address issue 529, fix incorrect markup for creating links in Asciido…
Dscano Dec 24, 2024
37a06b1
Proposal issue 525, Some odd syntax highlighting in some code blocks
Dscano Dec 24, 2024
4360049
Fix trailing whitespace
Dscano Dec 24, 2024
f19f1b6
Fix trailing whitespace
Dscano Dec 24, 2024
8c06ca7
Proposal issue 524, fix PDF numbered lists
Dscano Dec 25, 2024
0ed60c4
Proposal issue 523, fix PDF to avoid breaking bullet items, titles an…
Dscano Dec 25, 2024
ce784a1
Fix issue 522, removed negative indentation
Dscano Dec 26, 2024
ba93b11
Fix issue 527, rendering of inline code improved
Dscano Dec 26, 2024
a93198f
Centered images and table captions in HTML
Dscano Dec 28, 2024
d9aea99
Add LibreOffice to the Dockerfile and include a new make command.
Dscano Dec 30, 2024
79ab44a
Suggest a make command
Dscano Dec 30, 2024
2fc304a
Add to Makefile png and svg images generation
Dscano Jan 5, 2025
8b9c162
Add images dependencies to Makefile
Dscano Jan 5, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/any-branch-uploads.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
- uses: actions/checkout@v3
- name: Build spec
run: |
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-madoko:latest make
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-asciidoc :latest make
ls docs/v1/build
- name: Upload spec to S3 if needed
if: ${{ github.actor != 'dependabot[bot]' }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/main-branch-uploads.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
fetch-depth: 0
- name: Build spec
run: |
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-madoko:latest make
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-asciidoc :latest make
ls docs/v1/build
- name: Upload spec to S3
uses: jakejarvis/[email protected]
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,19 @@ on:
- '*-dev'

jobs:
madoko-lint:
asciidoc-lint:
runs-on: [ubuntu-latest]
steps:
- uses: actions/checkout@v3
- name: Run linter
run: |
./tools/madokolint.py docs/v1/P4Runtime-Spec.mdk
./tools/asciidoclint.py docs/v1/P4Runtime-Spec.adoc

build-spec:
runs-on: [ubuntu-latest]
steps:
- uses: actions/checkout@v3
- name: Build spec
run: |
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-madoko:latest make
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-asciidoc :latest make ci
ls docs/v1/build
2 changes: 1 addition & 1 deletion .github/workflows/tag-uploads.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
fetch-depth: 0
- name: Build spec
run: |
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-madoko:latest make
docker run -v `pwd`/docs/v1:/usr/src/p4-spec p4lang/p4rt-asciidoc :latest make
ls docs/v1/build
- name: Upload spec to S3
uses: jakejarvis/[email protected]
Expand Down
11 changes: 5 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ You can fork the repo and submit a pull request in Github.

All developers must have signed the [P4.org](http://p4.org) CLA.

### Madoko style checker
### AsciiDoc style checker

The P4Runtime specification is written using
[Madoko](http://madoko.org/reference.html). We provide a lint tool to catch
basic formatting issues and try to keep the spec uniform. The lint tool will be
run as part of CI and patches cannot be merged until it returns success. You can
run the lint tool locally with `./tools/madokolint.py`.
The P4Runtime specification is written using [AsciiDoc](https://docs.asciidoctor.org/).
We provide a lint tool to catch basic formatting issues and try to keep the spec uniform.
The lint tool will be run as part of CI and patches cannot be merged until it returns success. You can
run the lint tool locally with `./tools/asciidoclint.py`.
31 changes: 31 additions & 0 deletions docs/tools/Dockerfile.asciidoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
FROM ruby:3.3.5
LABEL maintainer="P4 API Working Group <[email protected]>"
LABEL description="Dockerfile used for building the asciidoc specification"

RUN apt-get update && \
apt-get install -y cmake flex bison libglib2.0-dev libcairo2-dev libpango1.0-dev libxml2-dev libwebp-dev libzstd-dev libgdk-pixbuf-2.0-dev time

RUN apt-get install -y libreoffice && \
gem install asciidoctor && \
echo 'gem: --no-document' > /etc/gemrc && \
gem install nokogiri && \
gem install rghost && \
gem install asciidoctor-diagram && \
gem install asciidoctor-plantuml && \
gem install asciidoctor-pdf --version 2.3.19 && \
gem install asciidoctor-pdf-cjk && \
gem install asciidoctor-lists --version 1.1.2 && \
gem install coderay pygments.rb thread_safe && \
gem install slim && \
gem install concurrent-ruby && \
gem install haml tilt && \
gem install asciidoctor-mathematical && \
gem install asciidoctor-bibtex &&\
git clone https://github.com/rouge-ruby/rouge &&\
cd rouge && \
git log -n 1 | cat && \
gem build rouge.gemspec && \
gem install rouge

VOLUME ["/usr/src/p4-spec"]
WORKDIR /usr/src/p4-spec
24 changes: 0 additions & 24 deletions docs/tools/Dockerfile.madoko

This file was deleted.

8 changes: 4 additions & 4 deletions docs/tools/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Dockerfile.madoko is used to build a Docker image which we use to render the
Dockerfile.asciidoc is used to build a Docker image which we use to render the
P4Runtime specification (HTML & PDF) in CI. The image can also be used locally
to build the specification without having to worry about installing all the
dependencies yourself.
Expand All @@ -9,9 +9,9 @@ simply pull the image from dockerhub and don't have to worry about building the
image themselves. If you are a maintainer and you need to upload a new version
of the Docker image to to dockerhub, you will need the following commands:
```bash
docker build -t p4rt-madoko -f Dockerfile.madoko .
docker tag p4rt-madoko p4lang/p4rt-madoko:latest
docker push p4lang/p4rt-madoko:latest
docker build -t p4rt-asciidoc -f Dockerfile.asciidoc .
docker tag p4rt-asciidoc p4lang/p4rt-asciidoc:latest
docker push p4lang/p4rt-asciidoc:latest
```

Note that you need to have write permissions to the p4lang dockerhub repository
Expand Down
35 changes: 23 additions & 12 deletions docs/v1/Makefile
Original file line number Diff line number Diff line change
@@ -1,21 +1,32 @@

SPEC=P4Runtime-Spec

all: html pdf build
ROUGE_STYLE=github
ROUGE_CSS=style

all: ${SPEC}.pdf ${SPEC}.html

build:
mkdir -p build

# Note: Brute-force image rendering; could use more precise file-by-file make rules
images: build assets/*.odg
soffice --convert-to svg --outdir build assets/*.odg > /dev/null 2>&1
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to restore lines like these in the Makefile, otherwise running make in the docs/v1 directory fails to find the .svg and .png image files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, we have 2 use cases:

  • If the Docker runs on a host machine, the user needs to generate the PNG images and then compile.
  • If the Docker has to run on a CI server, the images must be generated within the make process or checked in beforehand.

Copy link
Contributor

Choose a reason for hiding this comment

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

But we can reduce all cases to behave in a way where whether it is a bare metal system, a VM, or a container, the Makefile has the commands that generate the .svg/.png files from the OpenOffice source files, by installing LibreOffice on the system, yes? I thought that was the preference that Chris expressed earlier, and that is why we only have the OpenOffice files checked in, not the .svg/.png files.

Even if there are cases where the .svg/.png files are sitting there, running make with a Makefile that contains rules for generating those files will only generate them if they are needed, so I don't see only advantages to having these rules in the Makefile that can generate them.

Copy link
Contributor

@jafingerhut jafingerhut Dec 25, 2024

Choose a reason for hiding this comment

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

Sorry, one more quick comment, in case this is something I am confused about.

If there are any containers anywhere that anyone wants to use to generate the PDF & HTML for the P4Runtime spec, I think we should tell them: "Install LibreOffice in that container, or it won't work."

And if there is some container published by someone in the P4 community that you want to use, that installs asciidoctor, but not LibreOffice, I think we should ask them to update that container so that it does contain LibreOffice, even if LibreOffice is only needed for generating the P4Runtime spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Totally agree with you. Does this mean we need to implement it for the container running in the CI environment?

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like a very good idea to me that the container used by CI to build PDF and HTML should have LibreOffice installed, so that it can create the formats of image files required.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, good catch.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Dscano, can you please add back to the Makefile the rules for generating the .svg and .png files from the .odg files?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Dscano, can you please add back to the Makefile the rules for generating the .svg and .png files from the .odg files?

Sorry, I see you have added those rules back.

Can you please add images as a dependency of the generated PDF and HTML files, so that if the .svg and .png files do not currently exist on the file system, then running make with no target will run the commands that generate them, before generating PDF and HTML files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. So I added it

soffice --convert-to png --outdir build assets/*.odg > /dev/null 2>&1
${SPEC}.pdf: ${SPEC}.adoc
time asciidoctor-pdf -v \
-a pdf-fontsdir=resources/fonts \
-r asciidoctor-mathematical \
-r asciidoctor-bibtex \
-r asciidoctor-lists \
-a rouge-style=$(ROUGE_STYLE) $<

${SPEC}.html: ${SPEC}.adoc
time asciidoctor -v \
-r asciidoctor-mathematical \
-r asciidoctor-bibtex \
-r asciidoctor-lists \
-a rouge-css=$(ROUGE_CSS) $<

html: ${SPEC}.mdk p4.json assets/* images
madoko -vv --png --odir=build ${SPEC}.mdk
images:
soffice --convert-to png --outdir resources/figs resources/figs/*.odg > /dev/null 2>&1

Copy link
Contributor

Choose a reason for hiding this comment

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

svg are also generated by the current docs/v1/Makefile. Are they no longer necessary with AsciiDoc PDF and HTML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my understanding, no. As you can see in the .adoc file, the figures used are PNGs."

pdf: ${SPEC}.mdk p4.json assets/* images
madoko --pdf -vv --png --odir=build $<
ci: images all


clean:
${RM} -rf build
/bin/rm -f ${SPEC}.pdf ${SPEC}.html
Loading
Loading