-
Notifications
You must be signed in to change notification settings - Fork 90
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
base: main
Are you sure you want to change the base?
Changes from 22 commits
4f8de37
adcfbe9
7659239
780d780
c43d938
3d45a90
207dd87
f6703ed
ef538d6
4b9875f
dbbfe09
faf7275
4c48e1b
4e277de
36c2602
8f0f754
7fbb83d
50b8bf5
9eae1e4
6a58e48
d56cb14
3d6caa4
c0e0239
25118c6
71bc292
c2355be
02f0f21
37a06b1
4360049
f19f1b6
8c06ca7
0ed60c4
ce784a1
ba93b11
a93198f
d9aea99
79ab44a
2fc304a
8b9c162
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
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 | ||
|
||
RUN gem install asciidoctor \ | ||
gem install asciidoctor-pdf \ | ||
gem install asciidoctor-mathematical \ | ||
gem install asciidoctor-bibtex\ | ||
gem install asciidoctor-lists\ | ||
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 |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,21 +1,27 @@ | ||
|
||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, we have 2 use cases:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, good catch. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sorry, I see you have added those rules back. Can you please add There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
||
html: ${SPEC}.mdk p4.json assets/* images | ||
madoko -vv --png --odir=build ${SPEC}.mdk | ||
${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) $< | ||
|
||
pdf: ${SPEC}.mdk p4.json assets/* images | ||
madoko --pdf -vv --png --odir=build $< | ||
${SPEC}.html: ${SPEC}.adoc | ||
time asciidoctor -v \ | ||
-r asciidoctor-mathematical \ | ||
-r asciidoctor-bibtex \ | ||
-r asciidoctor-lists \ | ||
-a rouge-css=$(ROUGE_CSS) $< | ||
|
||
clean: | ||
${RM} -rf build | ||
/bin/rm -f ${SPEC}.pdf ${SPEC}.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the space between
asciidoc
and-lint
is causing this failure: https://github.com/p4lang/p4runtime/actions/runs/12320523043