Skip to content

Update LookPath in go-runhcs to account for changes in go 1.19+ - #1556

Closed
Kathryn Baldauf (katiewasnothere) wants to merge 1 commit into
mainfrom
kabaldau/fix_runhcs_lookpath_test
Closed

Update LookPath in go-runhcs to account for changes in go 1.19+#1556
Kathryn Baldauf (katiewasnothere) wants to merge 1 commit into
mainfrom
kabaldau/fix_runhcs_lookpath_test

Conversation

@katiewasnothere

@katiewasnothereKathryn Baldauf (katiewasnothere) commented Oct 31, 2022

Copy link
Copy Markdown

Signed-off-by: Kathryn Baldauf kabaldau@microsoft.com

Kathryn Baldauf
Update LookPath in go-runhcs to account for changes in go 1.19+
a3edaae
Signed-off-by: Kathryn Baldauf <kabaldau@microsoft.com>
@katiewasnothere

Kathryn Baldauf (katiewasnothere) commented Oct 31, 2022•
edited
Loading

Copy link
Copy Markdown
Author

Address CI build break that was revealed in #1554

@helsaawy

Hamza El-Saawy (helsaawy) commented Oct 31, 2022

Copy link
Copy Markdown
Contributor

ErrDot was introduced in 1.19, so this will fail to build for versions prior, right?
we could disable the new behavior with the env variable GODEBUG=execerrdot=0 in our build pipelines, or (probably easier), use https://pkg.go.dev/golang.org/x/sys/execabs#LookPath for the preceding linepath, err := exec.LookPath(command), and if that fails, use ./runhcs.exe instead...

jterry75
Justin (jterry75) reviewed Nov 1, 2022
View reviewed changes
Comment threadpkg/go-runhcs/runhcs.go
import (
"bytes"
"context"
"errors"

@jterry75Justin (jterry75)Nov 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is this package even used for anymore? I thought runhcs was dead? Arent we only using shim for containerd these days?

@katiewasnothereKathryn Baldauf (katiewasnothere)Nov 1, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Looks like it's mostly used in tests for runhcs, which we still use to create scratch disks unfortunately :/

@jterry75Justin (jterry75)Nov 1, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OMG. Create-scratch. What's LCOW 🙄 ? LOL. Ok fine

@katiewasnothere

Kathryn Baldauf (katiewasnothere) commented Nov 1, 2022

Copy link
Copy Markdown
Author

Hamza El-Saawy (@helsaawy) I think I'll just close this PR for now until we move to build with 1.19. Thoughts?

@jterry75

Justin (jterry75) commented Nov 1, 2022

Copy link
Copy Markdown
Contributor

LGTM

@helsaawy

Hamza El-Saawy (helsaawy) commented Nov 1, 2022

Copy link
Copy Markdown
Contributor

Hamza El-Saawy (@helsaawy) I think I'll just close this PR for now until we move to build with 1.19. Thoughts?

Fine by me; hopefully we dont run into the same issue elsewhere

@katiewasnothere

Kathryn Baldauf (katiewasnothere) commented Jan 24, 2024

Copy link
Copy Markdown
Author

Closing for now

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

Reviewers

@jterry75Justin (jterry75)jterry75 left review comments

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

@katiewasnothere@helsaawy@jterry75