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

proto, pkg, docs: update CSR docstrings #926

Merged
merged 12 commits into from
Jan 3, 2023

Conversation

woodruffw
Copy link
Member

Closes #863.

Signed-off-by: William Woodruff william@trailofbits.com

cc @haydentherapper

Signed-off-by: William Woodruff <william@trailofbits.com>
@@ -16,7 +16,7 @@
// Code generated by protoc-gen-go. DO NOT EDIT.
// versions:
// protoc-gen-go v1.28.1
// protoc v3.20.3
// protoc v3.21.11
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is gonna be an issue. My recommendation is to either update

version: '3.20.3'
, or install 3.20.3. We need to use a containerized protoc, we have an issue tracking this

Copy link
Member Author

Choose a reason for hiding this comment

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

My protoc is from brew, so downgrading might be a little painful 😅 -- I'll bump the action if that's alright.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, it seems to think that version doesn't exist.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah...looks like manifestation of this issue: arduino/setup-protoc#33

Copy link
Member Author

Choose a reason for hiding this comment

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

(It seems like protobuf changed its versioning to chop the 3. prefix off, and this action doesn't know about that.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I opened arduino/setup-protoc#58 with a fix to that action, and I've temporarily updated this PR to pin my trail-of-forks fork instead. But I assume we'll want this to be upstreamed before merging 🙂

Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw marked this pull request as draft December 16, 2022 21:28
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw
Copy link
Member Author

The diff is still failing, for reasons that aren't clear to me...is it possible protoc behaves differently on macOS vs. Linux in terms of comment generation?

@haydentherapper
Copy link
Contributor

@woodruffw Are you using Go 1.19.4? Differences in Go versions can also cause issues

@woodruffw
Copy link
Member Author

@woodruffw Are you using Go 1.19.4? Differences in Go versions can also cause issues

go version go1.18.2 darwin/amd64 according to go version, so pretty old 😅. I'll update and re-build.

Bumped my `go` version.

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw marked this pull request as ready for review January 3, 2023 22:12
@haydentherapper haydentherapper merged commit 2c62d06 into sigstore:main Jan 3, 2023
@woodruffw woodruffw deleted the ww/csr-docs branch January 3, 2023 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ambiguous CSR behavior: Fulcio accepts CSRs with email subjects that aren't emails
2 participants