-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 Github Actions for Ubuntu-24.04 #11112
base: master
Are you sure you want to change the base?
Conversation
@@ -52,7 +52,7 @@ func Release(lock int) error { | |||
|
|||
// CheckLock checks whether any process is using the lock | |||
func CheckLock(path string) bool { | |||
lockByte, _ := exec.Command("lsof", "-w", "-F", "ln", path).Output() | |||
lockByte, _ := exec.Command("lsof", "-w", "-F", "lfn", path).Output() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eugh, I wonder if we've ever checked to see if there is a pure go version of this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its literally only used in the unit test itself to validate the lock. The major lock libraries like https://github.com/gofrs/flock don't have a "checkLock" function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. It is a little weird that we have stuff that is only used by the test framework mixed in with the other flock package stuff; if we only need it in tests maybe it could go in the frameworks stuff. Just nitpicking though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, wouldn't TryLock()
cover what we're using CheckLock()
for?
Signed-off-by: Derek Nola <[email protected]>
Signed-off-by: Derek Nola <[email protected]>
Signed-off-by: Derek Nola <[email protected]>
Signed-off-by: Derek Nola <[email protected]>
Signed-off-by: Derek Nola <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11112 +/- ##
==========================================
- Coverage 49.91% 43.90% -6.02%
==========================================
Files 178 178
Lines 14820 14820
==========================================
- Hits 7398 6506 -892
- Misses 6070 7108 +1038
+ Partials 1352 1206 -146
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Proposed Changes
Fix lsof output for flock utility function. Ubuntu 24.04 moved to a new(ish) version of
lsof
which included Fix FD field description lsof-org/lsof#158.Fix composite action for libvirt/vagrant installation. Now works on ubuntu-24.04
Pinned e2e and unit test workflows to ubuntu-24.04 to prevent future disruption (in 2 years I can manually open PRs for ubuntu-26.04)
Types of Changes
Verification
Testing
Linked Issues
User-Facing Change
Further Comments