Split into separate CC/DWARF/ELF documents and convert to AsciiDoc #208
Conversation
|
LG. On a hindsight, it might be a good idea to split this into two steps: (a) split (b) convert to adoc. But you have already done the work, and splitting just adds more work when the time can be better spent on other stuff. |
|
Yeah, that was my original plan, then I realised that I'd either have to spend pointless effort making the build system work for the split Markdown documents or have an intermediate commit that was broken. I guess if I did things the other way round maybe it could have worked out. |
|
Why do we need our own fonts? I don't see fonts in other repos I have checked out. The cmu and mplus fonts are under the SIL license, which requires a copy of the license be included. The droid fonts are under the Apache license which also requires that a copy of the license be included. These license files are missing. There is a LICENSE.md file in the top level dir that implies that everything is under a Creative Commons license which is obviously incorrect for the fonts. But I think everything other than the fonts is creative commons licensed. This is potentially confusing. Poking around, I found riscv/docs-templates which seems to be the source of the problem. And the same fonts are used in other repos like riscv-CMOs with the same license confusion. I will file an issue with docs-templates. |
|
Indeed I just copied the template from docs-templates |
| :appendix-caption: Appendix | ||
| :imagesdir: images | ||
| :title-logo-image: image:risc-v_logo.png[pdfwidth=3.25in,align=center] | ||
| //:back-cover-image: image:backpage.png[opacity=25%] |
There was a problem hiding this comment.
Also, docs-templates has this uncommented but doesn't include this image, not sure what it's meant to do... (riscv/docs-dev-guide#7)
There was a problem hiding this comment.
Apologies, the image was missing. Should be fixed now.
We have been using Creative Commons for historical reasons only. It certainly doesn't make sense for example code or fonts. I'm working with folks more familiar with the topic of licensing to create a multi-license structure for our repos, and riscv/docs-templates will likely be the first repo to use this structure. |
|
An attempt to build it, after installing the asciidoctor-pdf package, got multiple errors. I fixed the coderay problem by installing the coderay package. For the $header error, in the resources/themes/risc-v_spec-pdf.yaml file I found a line which I had to change to I'll have to check to see if docs-templates is also broken. For the pencil icon, that is apparently part of font awesome, but installing both the fonts-font-awesome package and the ruby-font-awesome-rails package didn't solve it. Not obvious how to fix that. I commented out the line in resources/themes/risc-v_spec-pdf.yml that uses pencil-square-o. I'm now able to build the pdf file, getting multiple deprecation warnings for fa-info-circle, though I'm wondering how less capable people are expected to be able to build this. But I guess that is another issue for docs-templates. I see MacOS and Windows instructions there but no linux instructions. Maybe we should include a copy of the pdf file to make things easier for end users that just want to read the spec? |
|
How did you install asciidoctor-pdf? If with your system package manager, it probably has too old a version. I saw that same error with the GitHub Actions build trying to use the native Ubuntu 20.04 package (https://github.com/riscv/riscv-elf-psabi-doc/runs/3295180632) and fixed it by instead just installing asciidoctor{,-pdf} via gem to get a non-ancient version (https://github.com/riscv/riscv-elf-psabi-doc/actions/runs/1118233686). If you have ruby already installed, |
|
Also, we're already providing pre-built PDFs at https://github.com/riscv/riscv-elf-psabi-doc/releases (and deliberately not committed to the repo). |
|
(and for PRs the associated GitHub Actions check has the PDF as an artifact if you click on "Details" next to the build job, e.g. you can see the PDF for this PR at https://github.com/riscv/riscv-elf-psabi-doc/pull/208/checks?check_run_id=3295715615 in the top right) |
|
Reading this to review it, I noticed that neither your name nor mine is in the list of contributors at the top. Maybe we should update that. And maybe include maskrays real name too. I find putting the code model info in the ELF section odd. And putting C++ name mangling under code models even odder. Maybe the first section could be renamed from Calling Conventions to Compiler Conventions and then it can include code model, and C++ name mangling. Then the second section could be Linker Conventions and has the ELF info and Relaxation infon. And the last section can be debugger conventions which includes the dwarf info. The relocation table in section 5.5 is poorly formatted. About half of the relocation names are printed across two lines which makes them hard to read. At least in my linux produced pdf file. They all fit on one line in the old md version of the doc. The attribute table in section 5.12.1 has the same problem. None of this is stuff that has to be fixed now. They can all be fixed after the conversion if we want to make the changes. |
Yeah, I had noticed that was rather outdated (should've filed an issue tracking that like I did for some other things I noticed when sweeping through the whole document... being forced to convert it is one way to make oneself review the whole thing). Hadn't noticed the point about MaskRay, we should fix that too indeed.
The issue with code models is that macOS and Windows have quite different linkage models. Looking at what they do for AArch64 briefly I think it may be fine to specify the code models as universal and that they do carry across despite the different linkage models, but mention of the GOT is invalid for Windows, and I'm not sure if it has an equivalent to undefined weak symbols.
The markdown version was even worse, it gave things like:
I agree the formatting still needs some work to make it more readable, but it's at least an improvement over that horror...
Agreed; this was intended to be as NFC as possible. |
|
I'm on Ubuntu 20.04 and using apt install to get packages. I haven't tried using gem yet, but did see comments about it when I was looking for asciidoctor info. I didn't think to check for a release, it is good that the pdf file is available there. Maybe we should add a comment to the README.md file to point people at the pdf file in the releases area. Trying gem now, it does work better. I didn't have to modify the themes yaml file. I get the same warnings for deprecated fa fonts, now info messages. And a warning that is maybe for a misplaced newline character. Apt gave me version 1.5/2.0.10 and gem gave me 1.6/2.0.16 so not a major difference. |
|
Thank you for this PR. |
I've filed #211 and #212 to make sure we don't lose track of the first and third sets of points. For the middle one, I don't think your proposal is quite right either, but there's certainly scope to rename and shuffle things around further, especially now that discussions about how the various non-ISA documents are going to be combined have just started. |
|
(and the README now points people at the releases for PDFs) |
The repository name and structure in the RISC-V GitHub org has been modified, rendering the existing link invalid. This updates to point at the new location of the RISC-V DWARF specification. Change occured in riscv-non-isa/riscv-elf-psabi-doc#208 Change-Id: I8ca4c390bee2d7ce20418cdd00e4945a426cf5f7 Reviewed-on: https://go-review.googlesource.com/c/go/+/363355 Reviewed-by: Brad Fitzpatrick <[email protected]> Trust: Brad Fitzpatrick <[email protected]> Trust: Than McIntosh <[email protected]>
Closes: #204