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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Update command #9

Merged
merged 14 commits into from
Apr 4, 2020
Merged

Feature: Update command #9

merged 14 commits into from
Apr 4, 2020

Conversation

pxtxs
Copy link
Contributor

@pxtxs pxtxs commented Mar 6, 2020

Fixes #7
I'm new to Rust so please review carefully and tell me if it needs any changes.
馃憤

@pxtxs
Copy link
Contributor Author

pxtxs commented Mar 6, 2020

@sbstp

src/cmd/update.rs Outdated Show resolved Hide resolved
src/cmd/update.rs Outdated Show resolved Hide resolved
src/cmd/update.rs Outdated Show resolved Hide resolved
src/cmd/update.rs Outdated Show resolved Hide resolved
src/cmd/update.rs Outdated Show resolved Hide resolved
@pxtxs
Copy link
Contributor Author

pxtxs commented Mar 8, 2020

@sbstp Thanks for this review. Just did a new commit with some rewriting :)

src/cmd/update.rs Outdated Show resolved Hide resolved
@pxtxs pxtxs changed the title Feature: Update command WIP: Feature: Update command Mar 10, 2020
@pxtxs
Copy link
Contributor Author

pxtxs commented Mar 10, 2020

@sbstp I put this PR on hold for now, I need to change the way I get the latest version to a more reliable way. It wont work as soon as the project reach a 2 digits version number.

@pxtxs pxtxs changed the title WIP: Feature: Update command Feature: Update command Apr 4, 2020
src/cmd/update.rs Outdated Show resolved Hide resolved
src/cmd/update.rs Outdated Show resolved Hide resolved
src/cmd/update.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated
serde = { version = "1", features = ["derive"] }
serde_json = "1.0"
Copy link
Owner

Choose a reason for hiding this comment

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

Can you change this to "1", semver should protect us from breaking changes.

Cargo.toml Outdated
anyhow = "1"
atty = "0.2"
dirs = "2"
glob = "0.3"
lazy_static = "1"
libc = "0.2"
os_info = "2.0.2"
Copy link
Owner

Choose a reason for hiding this comment

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

Can you change this to "2", semver protects us from breaking changes.

Cargo.toml Outdated
@@ -13,13 +13,16 @@ license = "Zlib"
exclude = ["releases/**/*"]

[dependencies]
attohttpc = { version = "0.11.1", features = ["json"] }
Copy link
Owner

Choose a reason for hiding this comment

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

0.12 is out

@sbstp
Copy link
Owner

sbstp commented Apr 4, 2020

I tried out the self update but I have an error:

simon@smith:~/projects/kubie$ target/debug/kubie update
A new version of Kubie is available (v0.7.3), the new version will be automatically installed...
Error: Update failed. Consider using sudo?

Caused by:
    Invalid cross-device link (os error 18)

@sbstp
Copy link
Owner

sbstp commented Apr 4, 2020

I did some googling, the error above might be due to renaming across filesystems. We put the downloaded binary in /tmp and then rename it to some other place not in /tmp. /tmp is a special file system.

@pxtxs
Copy link
Contributor Author

pxtxs commented Apr 4, 2020

Interesting, I did not encounter this error during my tests. I'll check it out.

@pxtxs
Copy link
Contributor Author

pxtxs commented Apr 4, 2020

@sbstp Could you try to reproduce this issue ? It's working on my MacOS env and in my Linux VM

@sbstp
Copy link
Owner

sbstp commented Apr 4, 2020

Yes, I'll check.

@sbstp
Copy link
Owner

sbstp commented Apr 4, 2020

simon@smith:~/projects/kubie$ target/debug/kubie update
A new version of Kubie is available (v0.7.3), the new version will be automatically installed...
Error: Update failed. Consider using sudo?

Caused by:
    Text file busy (os error 26)

I think the file must be unlinked/removed before we copy over it.

@pxtxs
Copy link
Contributor Author

pxtxs commented Apr 4, 2020

I updated the code to remove the file before trying the copy.
You were right about the different filesystems => https://doc.rust-lang.org/src/std/fs.rs.html#1654

@sbstp
Copy link
Owner

sbstp commented Apr 4, 2020

It works!

@pxtxs
Copy link
Contributor Author

pxtxs commented Apr 4, 2020

Be sure to squash lol if we are done with this PR

@sbstp sbstp merged commit 64b0c1b into sbstp:master Apr 4, 2020
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.

Feature request : Auto-update
2 participants