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

Add mechanism to resolve updates #46

Merged
merged 15 commits into from
May 7, 2024
Merged

Add mechanism to resolve updates #46

merged 15 commits into from
May 7, 2024

Conversation

hinerm
Copy link
Collaborator

@hinerm hinerm commented May 3, 2024

Fixes #3.

@hinerm hinerm requested a review from ctrueden May 3, 2024 19:49
@hinerm
Copy link
Collaborator Author

hinerm commented May 3, 2024

@ctrueden do you mind testing this on a non-Windows system? I just made a new file in the /update structure and verified it was copied over to the equivalent location, and that an empty file in /update would lead to the corresponding file deletion.

We're currently missing special logic to handle updating launcher files 😬 any thoughts there?

Copy link
Member

@ctrueden ctrueden left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! 👍

Do you think we can get away with not having an explicit stat() function?

src/commonMain/kotlin/main.kt Outdated Show resolved Hide resolved
src/commonMain/kotlin/file.kt Outdated Show resolved Hide resolved
src/commonMain/kotlin/main.kt Outdated Show resolved Hide resolved
src/windowsMain/kotlin/file.kt Show resolved Hide resolved
These file manipulation functions are necessary to complete ImageJ-style
application updates.
@ctrueden
Copy link
Member

ctrueden commented May 4, 2024

I rebased over the latest main branch, and changed move to mv. I may look into the stat versus length thing later this weekend. If not, we can check it on Monday.

@ctrueden
Copy link
Member

ctrueden commented May 4, 2024

@hinerm I updated the File class to have a length field now instead of a stat() function. I also tidied up the Windows code such that length and lines() share common code as appropriate. I tested on Linux and all is working. Could you please test on Windows? If it still works on Windows, we can merge it!

@ctrueden
Copy link
Member

ctrueden commented May 5, 2024

I did find one limitation in my tests: if there is a file in the update folder inside a directory that does not exist in the main directory structure, the logic does not create the matching directory first, so the mv operation fails. We can remedy this by doing a mkdir (recursively as needed) before moving the file.

@hinerm
Copy link
Collaborator Author

hinerm commented May 6, 2024

All seems good on windows

hinerm and others added 7 commits May 6, 2024 16:10
This assumes there is an "update" app subdirectory with a directory
structure shadowing that of the main app. Empty update files signify the
corresponding app file should be deleted; otherwise files are copied
from the update structure to their equivalent app location. The update
directory and any remaining contets are deleted.

Co-authored-by: Curtis Rueden <[email protected]>
And make the Windows File code more DRY between length and lines().
The function will always be there on Windows.
And if it's not, we'll now fail with NPE instead of RuntimeException.
Either way it's a crash. *shrug*
Necessary in case there are files in a new subdirectory structure.
@hinerm
Copy link
Collaborator Author

hinerm commented May 6, 2024

@ctrueden ok, making empty intermediate directories should work now. Anything else before merging?

@hinerm
Copy link
Collaborator Author

hinerm commented May 6, 2024

 Task :compileKotlinMacosX64 FAILED
e: file:///Users/runner/work/jaunch/jaunch/src/posixMain/kotlin/file.kt:89:29 Type mismatch: inferred type is UInt but mode_t /* = UShort */ was expected

Nuh uh.... ☹️
image

@ctrueden supposedly this was fixed in 1.6.0 so I don't know why it's a problem?

So that we can have a separate mkdir function for each,
without replicating the code across x64 and arm64.
It's more succinct, and just as clear (to us ;-).
@ctrueden
Copy link
Member

ctrueden commented May 7, 2024

Whew! Finally working on all platforms. 😌

@ctrueden ctrueden merged commit c7dd816 into main May 7, 2024
4 checks passed
@ctrueden ctrueden deleted the resolve-updates branch May 7, 2024 17:56
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.

Pending updates
2 participants