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

Fix Windows embedded file path bug #103

Merged

Conversation

malancas
Copy link
Collaborator

@malancas malancas commented Feb 20, 2024

Summary

This fixes a Windows compatibility issue related to embedded file paths and a bug downloading target metadata in the go-tuf library.

Fixes #102

Release Note

Documentation

malancas and others added 7 commits February 15, 2024 15:05
Signed-off-by: Meredith Lancaster <[email protected]>
Signed-off-by: Meredith Lancaster <[email protected]>
Signed-off-by: Meredith Lancaster <[email protected]>
Signed-off-by: Fredrik Skogman <[email protected]>
on other platforms such as windows

Signed-off-by: Fredrik Skogman <[email protected]>
@malancas malancas marked this pull request as ready for review February 22, 2024 16:02
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Build action is stuck

@malancas
Copy link
Collaborator Author

Build action is stuck

I'll take a look

@kommendorkapten
Copy link
Member

On the stuck build job @malancas wrote this to me:

I see the build workflow is hanging on the PR, I'm guessing it has to do with the new matrix jobs and the build job not matching the required checks list anymore

And the branch protection rule requires a job named build to run https://github.com/sigstore/community/blob/main/github-sync/github-data/sigstore/repositories.yaml#L1860
And we can see that in the list above.

Can that be the issue, as it does say: 1 expected check, but there is no link to any details. The build jobs from the build with a matrix are reported as "Build and test/build ()".

How about renaming the build job to build-and-test then create a new job build, that does nothing but requires all the build from build-and-test?

Signed-off-by: Meredith Lancaster <[email protected]>
…sigstore-go into embedded-file-path-windows-bug
@malancas
Copy link
Collaborator Author

The required cosign status checks account for platform related matrix jobs: https://github.com/sigstore/community/blob/main/github-sync/github-data/sigstore/repositories.yaml#L228-L230. We could update the sigstore-go required checks entry to match this.

@haydentherapper
Copy link
Contributor

Ah good catch, yea we'll need to update the checks in the community repo.

@malancas
Copy link
Collaborator Author

malancas commented Feb 22, 2024

I'll open a PR for that now

@@ -16,7 +16,7 @@ require (
github.com/sigstore/sigstore v1.8.1
github.com/sigstore/timestamp-authority v1.2.1
github.com/stretchr/testify v1.8.4
github.com/theupdateframework/go-tuf/v2 v2.0.0-20240207172116-f5cf71290141
github.com/theupdateframework/go-tuf/v2 v2.0.0-20240222081530-454b12158917
Copy link
Contributor

Choose a reason for hiding this comment

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

@kommendorkapten not a blocker, but do you know when go-tuf maintainers will be cutting a release with the v2 implementation?

Copy link
Member

Choose a reason for hiding this comment

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

@haydentherapper there is still one open bug for Windows in go-tuf that I intend to fix today. When all test cases are working for all platforms, and we have a clear understanding of the current open issues, I think it's time we create a first release. I would guess one-two weeks from now?

@malancas
Copy link
Collaborator Author

When someone has time to review: sigstore/community#408

Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Should be good now. Once merged we'll just need to rebase other PRs

@haydentherapper haydentherapper merged commit d0d98c7 into sigstore:main Feb 22, 2024
10 checks passed
@malancas malancas deleted the embedded-file-path-windows-bug branch February 22, 2024 22:10
Copy link

@alphachart alphachart left a comment

Choose a reason for hiding this comment

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

.

Copy link

@alphachart alphachart left a comment

Choose a reason for hiding this comment

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

.

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.

Embedded file system file path creation fails on Windows
4 participants