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

chore: avoid apparent race condition in Make task execution #23

Merged
merged 2 commits into from
Jan 19, 2024

Conversation

ctreatma
Copy link
Contributor

This is a weird one...somehow, switching to running Go commands in a container (#14), combined with the passage of time (worked fine for me last week, consistently doesn't this week) has broken the make generate-all task (and the underlying make -f Makefile.metalv1 generate task). But only when I'm running locally.

In CI, make generate-all runs just fine (as it used to do for me locally). Today, locally, it fails every time. After digging through loads of console output I found this message about halfway through:

docker  run --rm -u 502:20 -v /Users/ctreatman/Documents/code/equinix-sdk-go:/workdir -w /workdir -e GOCACHE=/tmp/.cache golang:1.19 go mod tidy
go: warning: "all" matched no packages

As a result of this failed execution, the go.mod and go.sum don't declare any dependencies, which eventually causes the Go tests to fail.

Makefile Outdated
@@ -35,7 +35,7 @@ mod:
${GO_CMD} go mod init github.com/${GIT_ORG}/${GIT_REPO}
${GO_CMD} go mod tidy

test:
test: mod
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a first pass I've made mod a prerequisite for test and removed it from the prerequisites list for generate in the SDK Makefile(s). This works for now but I think the actual race involves mod running before remove-unused has finished removing the generated <service>/go.(mod|sum) files. A more direct fix might be to have the mod task remove generated service-specific go.mod and go.sum files instead.

Copy link
Member

Choose a reason for hiding this comment

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

Just checked this out and tried building on my linux server, seems to run fine at least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up updating the mod task to clean up the extra go.mod and go.sum files and reordering the prerequisites for generate and that approach seems to work to. Let me know if you prefer the earlier version of this PR instead, though.

Copy link
Member

Choose a reason for hiding this comment

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

No, I prefer this way.

@cprivitere cprivitere marked this pull request as ready for review January 17, 2024 21:45
@cprivitere cprivitere marked this pull request as draft January 17, 2024 21:45
@ctreatma ctreatma marked this pull request as ready for review January 17, 2024 23:06
@ctreatma ctreatma changed the title fix: avoid apparent race condition in task execution chore: avoid apparent race condition in Make task execution Jan 19, 2024
Copy link
Member

@cprivitere cprivitere left a comment

Choose a reason for hiding this comment

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

lgtm

@cprivitere cprivitere merged commit 9e6dfd4 into main Jan 19, 2024
2 checks passed
@cprivitere cprivitere deleted the fix-task-execution branch January 19, 2024 18:32
Copy link
Contributor

This PR is included in version 0.31.2 🎉

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.

2 participants