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 upgrade command on some Linux systems #290

Merged
merged 2 commits into from
Feb 21, 2016

Conversation

jbaiter
Copy link
Contributor

@jbaiter jbaiter commented Feb 3, 2016

Previously the still open executable file was directly written to, which caused the program to abort with a text file busy message on some systems.

Now the following approach is taken:

  1. Move the old file to a backup path
  2. Try to write to the original path
  3. If opening/writing the original path fails: Move backup to old location
  4. If upgrade is successful: Remove backup file

Previously the still open executable file was directly written to, which
caused the program to abort with a `text file busy` message on some
systems. Now we remove the file before we write to it, which does not
cause any issues.
@kotp
Copy link
Member

kotp commented Feb 3, 2016

It really helps to have the "fixes #288" in the body of the commit as compared to the title or summary as it links there in the web gui. This is more a complaint to Github itself than anyone that puts that in the title though.

@jbaiter jbaiter changed the title Fix upgrade command on some Linux systems (fixes #288) Fix upgrade command on some Linux systems Feb 3, 2016
@jbaiter
Copy link
Contributor Author

jbaiter commented Feb 3, 2016

Sorry for that, I somewhat mindlessly retyped the original commit title.

@kotp
Copy link
Member

kotp commented Feb 4, 2016

It's the functionality of that statement, when combined into master branch, it will auto-close the issue, so it is useful. Unsure if it closes from the first line or not, but in the body on the web interface, it links. So a couple of functional things for having it. I think it should link in the title as well, not just in the body, but that may just be me.

@Tonkpils
Copy link
Contributor

Tonkpils commented Feb 4, 2016

Thanks for this PR! I'll take a look at it soon.

@Tonkpils
Copy link
Contributor

I tested this on Linux and OSX and works great

Tonkpils added a commit that referenced this pull request Feb 21, 2016
Fix `upgrade` command on some Linux systems
@Tonkpils Tonkpils merged commit 8a0fb5e into exercism:master Feb 21, 2016
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.

3 participants