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: add a build.lock file to gate the buildengine #1670

Merged
merged 5 commits into from
Jun 11, 2024
Merged

Conversation

safeer
Copy link
Contributor

@safeer safeer commented Jun 5, 2024

Fixes #1636

@safeer safeer changed the title fix: add a build.lock file to gate the buildengine, fixes #1636 fix: add a build.lock file to gate the buildengine Jun 5, 2024
@ftl-robot ftl-robot mentioned this pull request Jun 5, 2024
cmd/ftl/cmd_dev.go Outdated Show resolved Hide resolved
@safeer safeer force-pushed the build-lock-file branch from a9abd9b to c2a3f06 Compare June 5, 2024 22:00
@safeer safeer force-pushed the build-lock-file branch from c2a3f06 to 8bc165c Compare June 10, 2024 18:38
@safeer safeer force-pushed the build-lock-file branch from 8bc165c to 16594c8 Compare June 10, 2024 18:54
@safeer safeer marked this pull request as ready for review June 10, 2024 18:55
@safeer safeer requested a review from alecthomas as a code owner June 10, 2024 18:55
@safeer safeer requested review from a team, matt2e and wesbillman and removed request for a team June 10, 2024 18:55
@safeer safeer marked this pull request as draft June 10, 2024 20:23
Copy link
Collaborator

@alecthomas alecthomas left a comment

Choose a reason for hiding this comment

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

Looks good, interesting that one of the tests is failing...

buildengine/engine.go Outdated Show resolved Hide resolved
return nil, err
}
lock := flock.New(lockFile)
locked, err := lock.TryLock()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ohhh flock is cool :)

@safeer safeer marked this pull request as ready for review June 10, 2024 22:03
@safeer safeer merged commit e711523 into main Jun 11, 2024
39 checks passed
@safeer safeer deleted the build-lock-file branch June 11, 2024 00:11
alecthomas added a commit that referenced this pull request Jun 11, 2024
This reverts commit e711523.

```
=== Failed
=== FAIL: buildengine TestDissallowMultipleEngines (0.00s)
    /home/runner/work/ftl/ftl/buildengine/engine_test.go:65: Did not expect an error but got:
        failed to lock file at: /home/runner/.ftl/build.lock

DONE 383 tests, 1 skipped, 1 failure in 115.533s
Error: === RUN   TestDissallowMultipleEngines
    buildengine/engine_test.go:65: Did not expect an error but got:
        failed to lock file at: /home/runner/.ftl/build.lock
```
alecthomas added a commit that referenced this pull request Jun 11, 2024
…1732)

This reverts commit e711523.

```
=== Failed
=== FAIL: buildengine TestDissallowMultipleEngines (0.00s)
    /home/runner/work/ftl/ftl/buildengine/engine_test.go:65: Did not expect an error but got:
        failed to lock file at: /home/runner/.ftl/build.lock

DONE 383 tests, 1 skipped, 1 failure in 115.533s
Error: === RUN   TestDissallowMultipleEngines
    buildengine/engine_test.go:65: Did not expect an error but got:
        failed to lock file at: /home/runner/.ftl/build.lock
```

cc @safeer
@matt2e matt2e added the approved Marks an already closed PR as approved label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Marks an already closed PR as approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add locking such that no two build tasks can run at the same time
4 participants