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(build): kill chg daemon before installing #939

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vegerot
Copy link
Contributor

@vegerot vegerot commented Aug 25, 2024

Summary:
Frequently, if I have sl running when trying to upgrade sl, I get this error
during make install-oss:

$ make install-oss
...
rm -f sl
cp build/scripts-3.11/sl sl
mkdir -p //usr/local/bin
cp sl //usr/local/bin
cp: cannot create regular file '//usr/local/bin/sl': Text file busy
make: *** [Makefile:99: install-oss] Error 1

This diff works around the issue by killing the chg daemon before installing
the new version of sl.

I also added a sleep 1 after killing the chg daemon, because I was still
seeing the issue occasionally without it. I really dislike adding this sleep,
so I'd appreciate any suggestions

Test Plan:

This happens to me about 75% of the time I try to upgrade sl, but I don't have
reliable reproduction steps

Summary:
Frequently, if I have sl running when trying to upgrade sl, I get this error
during `make install-oss`:

```sh
$ make install-oss
...
rm -f sl
cp build/scripts-3.11/sl sl
mkdir -p //usr/local/bin
cp sl //usr/local/bin
cp: cannot create regular file '//usr/local/bin/sl': Text file busy
make: *** [Makefile:99: install-oss] Error 1
```

This diff works around the issue by killing the chg daemon before installing
the new version of sl.

I also added a `sleep 1` after killing the chg daemon, because I was still
seeing the issue occasionally without it.  I really dislike adding this sleep,
so I'd appreciate any suggestions

Test Plan:

This happens to me about 75% of the time I try to upgrade sl, but I don't have
reliable reproduction steps
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@quark-zju
Copy link
Contributor

Perhaps you can rm the destination before cp? That does not require killing processes (which might be surprising).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants