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.

Sign up for GitHub

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

Jump to bottom

Wavedrom updates - machine.adoc #1482

Merged
merged 8 commits into from
Jul 2, 2024
Merged

Wavedrom updates - machine.adoc #1482

merged 8 commits into from
Jul 2, 2024

Conversation

kersten1
Copy link
Collaborator

Fixing up wavedrom things one chapter at a time. Weirdly started with machine.adoc. Just roll with it.

ved-rivos
ved-rivos reviewed Jun 28, 2024
View reviewed changes
src/images/wavedrom/trap-return.adoc
{bits: 5, name: 'rd'},
{bits: 3, name: 'funct3'},
{bits: 5, name: 'rs1'},
{bits: 12, name: 'funct12'},
], config: {bits: 32}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

While removing the type tag this update inadvertently also removes the attribute tag - the attribute tag should be retained.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you! added them back in.

aswaterman
aswaterman approved these changes Jun 28, 2024
View reviewed changes
Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

I didn't perform a detailed code review, but I certainly approve in principle!

@kersten1 kersten1 merged commit b7a445a into main Jul 2, 2024
2 checks passed
@kersten1 kersten1 deleted the wavedrom-updates branch July 2, 2024 01:06
@aswaterman
Copy link
Member

aswaterman commented Jul 2, 2024 •
edited
Loading

@kersten1 this PR seems to have broken the build. (I'm using the latest docker image.)

asciidoctor: ERROR: images/bytefield/mideleg.adoc: line 2: Failed to generate image: undefined method `[]' for nil:NilClass
asciidoctor: ERROR: images/bytefield/sie.edn: line 2: Failed to generate image: undefined method `[]' for nil:NilClass
asciidoctor: ERROR: hypervisor.adoc: line 587: Failed to generate image: unexpected token at ''

@kersten1
Copy link
Collaborator Author

kersten1 commented Jul 2, 2024

@aswaterman I can't imagine why as I didn't touch those files at all. Looking.... Although I'm not the best at debugging builds yet....

@aswaterman
Copy link
Member

It looks like there is a concurrency problem with parallel builds; make -j1 works but make -j8 fails. Most likely, the addition of more wavedrom figures increased the probability of failure. So, not your fault. I'll see if I can figure out what to do about this.

@aswaterman
Copy link
Member

OK, it looks like the issue is that the wavedrom builds are creating temporary files inside the source tree, and so there is a race condition between multiple builds. The easy fix is to create a temporary copy of the source tree for each individual build. I'll handle it.

@aswaterman
Copy link
Member

BTW @kersten1 sorry for the false alarm about this PR; you were just unlucky enough to step on this pre-existing land mine :)

@kersten1
Copy link
Collaborator Author

kersten1 commented Jul 3, 2024

@aswaterman I just felt bad that I was useless at debugging.... Bill is back tomorrow and can help too.

@aswaterman
Copy link
Member

No prob. Fixed in #1491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ved-rivos ved-rivos ved-rivos left review comments

@aswaterman aswaterman aswaterman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kersten1 @aswaterman @ved-rivos