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.

Sign up for GitHub

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

Jump to bottom

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 •
edited by ctrueden
Loading

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?

ctrueden
ctrueden requested changes May 3, 2024
View reviewed changes
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
@hinerm @ctrueden
Add mv, rm, rmdir and stat to File
46d3403 
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
@hinerm @ctrueden
Add update logic to configurator
111fd03 
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]>
@hinerm
Enable updating in Fiji if JVM directive is active
dc9577e
@ctrueden @hinerm
Change File.stat() function to File.length
3d65094 
And make the Windows File code more DRY between length and lines().
@ctrueden @hinerm
Make file mv/rm error handling more concise
06cad80
@ctrueden @hinerm
Make GetFullPathName function use more succinct
4b92a2a 
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*
@hinerm
Add File.mkdir method
fc67fe0
@hinerm
Run mkdir before copying new files over
76a9eee 
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 •
edited
Loading 

 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?

@ctrueden
Split out macOS and Linux base platform targets
88fedac 
So that we can have a separate mkdir function for each,
without replicating the code across x64 and arm64.
@ctrueden
Rename install-pending-updates to apply-update
e9360f3 
It's more succinct, and just as clear (to us ;-).
@ctrueden
In debug mode, show update file system commands
07e49bc
@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
@ctrueden ctrueden mentioned this pull request Sep 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ctrueden ctrueden ctrueden requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pending updates
2 participants
@hinerm @ctrueden