Skip to content

Add apps support #964

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

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Add apps support #964

wants to merge 9 commits into from

Conversation

davidanthoff
Copy link
Collaborator

@davidanthoff davidanthoff commented Jun 20, 2024

More a proof of concept at the moment.

Definition of an app looks like https://github.com/davidanthoff/examplejlapp.

To add an app:

juliaup app add https://raw.githubusercontent.com/davidanthoff/examplejlapp/main/

To run the app:

juliaup app run jlappexe1 arg1 arg2
juliaup app run jlappexe2 arg1 arg2

To remove the app:

juliaup app remove ExampleJlTestApp

Lots and lots of things that would need to be added:

  • Handling of even the most basic errors :)
  • Shims
  • Registry story
  • Updates
  • Status
  • Maybe the app commands should actually have their own app, like jlapp add ...
  • Handle installs of apps into different depots

@davidanthoff
Copy link
Collaborator Author

@simeonschaub
Copy link
Member

Are you aware of JuliaLang/Pkg.jl#3772? I think this would be good to coordinate with that PR so we don't have two completely separate notions of apps in the Julia ecosystem

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit

rustfmt

[rustfmt] reported by reviewdog 🐶

let julia_binary_path = &paths.juliaupconfig


[rustfmt] reported by reviewdog 🐶

.join(config_file.data.installed_versions.get(&foo.2).unwrap().path.clone())


[rustfmt] reported by reviewdog 🐶

.normalize().unwrap();


[rustfmt] reported by reviewdog 🐶

}
else {


[rustfmt] reported by reviewdog 🐶

return Ok(())


[rustfmt] reported by reviewdog 🐶

pub target: String


[rustfmt] reported by reviewdog 🐶

execution_aliases: HashMap<String, JuliaupConfigExcutionAlias>


[rustfmt] reported by reviewdog 🐶

pub mod command_app_run;


[rustfmt] reported by reviewdog 🐶

pub mod command_completions;


[rustfmt] reported by reviewdog 🐶

juliaup/src/operations.rs

Lines 229 to 233 in 0fc1554

pub fn download_file(
url: &str,
target_path: &Path,
filename: &str
) -> Result<()> {


[rustfmt] reported by reviewdog 🐶

let mut content = std::io::Cursor::new(response.bytes().unwrap());


[rustfmt] reported by reviewdog 🐶

juliaup/src/operations.rs

Lines 246 to 250 in 0fc1554

pub fn download_file(
url: &str,
target_path: &Path,
filename: &str
) -> Result<String> {


[rustfmt] reported by reviewdog 🐶

juliaup/src/operations.rs

Lines 280 to 281 in 0fc1554

let folder = windows::Storage::StorageFolder::GetFolderFromPathAsync(&HSTRING::from(target_path)).unwrap().get().unwrap();
let file = folder.CreateFileAsync(&HSTRING::from(filename), windows::Storage::CreationCollisionOption::ReplaceExisting).unwrap().get().unwrap();


[rustfmt] reported by reviewdog 🐶

let stream = file.OpenAsync(FileAccessMode::ReadWrite).unwrap().get().unwrap();


[rustfmt] reported by reviewdog 🐶

http_response_content.WriteToStreamAsync(&stream).unwrap().get().unwrap();


[rustfmt] reported by reviewdog 🐶

juliaup/src/operations.rs

Lines 833 to 834 in 0fc1554

JuliaupConfigApplication::DevedApplication { path: _, julia_version, julia_depot: _, execution_aliases: _ } => julia_version != installed_version
} ) {

use juliaup::command_api::run_command_api;
use juliaup::command_app_add::run_command_app_add;
use juliaup::command_app_run::run_command_app_run;
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
use juliaup::command_app_run::run_command_app_run;

use juliaup::command_api::run_command_api;
use juliaup::command_app_add::run_command_app_add;
use juliaup::command_app_run::run_command_app_run;
use juliaup::command_app_remove::run_command_app_remove;
use juliaup::command_completions::run_command_completions;
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
use juliaup::command_completions::run_command_completions;
use juliaup::command_app_run::run_command_app_run;
use juliaup::command_completions::run_command_completions;

Comment on lines +156 to +162
ApplicationSubCmd::Add { value } => {
run_command_app_add(&value, &paths)
},
ApplicationSubCmd::Run { name, args } => {
run_command_app_run(&name, &args, &paths)
},
ApplicationSubCmd::Remove { name } => run_command_app_remove(&name, &paths)
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
ApplicationSubCmd::Add { value } => {
run_command_app_add(&value, &paths)
},
ApplicationSubCmd::Run { name, args } => {
run_command_app_run(&name, &args, &paths)
},
ApplicationSubCmd::Remove { name } => run_command_app_remove(&name, &paths)
ApplicationSubCmd::Add { value } => run_command_app_add(&value, &paths),
ApplicationSubCmd::Run { name, args } => run_command_app_run(&name, &args, &paths),
ApplicationSubCmd::Remove { name } => run_command_app_remove(&name, &paths),

Comment on lines +169 to +171
Add {
value: String
},
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
Add {
value: String
},
Add { value: String },

Comment on lines +174 to +177
Run {
name: String,
args: Vec<String>
},
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
Run {
name: String,
args: Vec<String>
},
Run { name: String, args: Vec<String> },

Comment on lines +3 to +4
use crate::{config_file::{load_config_db, JuliaupConfigApplication}, global_paths::GlobalPaths};
use anyhow::{Context,Result};
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
use crate::{config_file::{load_config_db, JuliaupConfigApplication}, global_paths::GlobalPaths};
use anyhow::{Context,Result};
use crate::{
config_file::{load_config_db, JuliaupConfigApplication},
global_paths::GlobalPaths,
};
use anyhow::{Context, Result};

use normpath::PathExt;

pub fn run_command_app_run(name: &str, args: &Vec<String>, paths: &GlobalPaths) -> Result<()> {

Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change

let config_file = load_config_db(paths)
.with_context(|| "`app run` command failed to load configuration data.")?;

let target: HashMap<String,(String,String,String,String)> = config_file
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
let target: HashMap<String,(String,String,String,String)> = config_file
let target: HashMap<String, (String, String, String, String)> = config_file

Comment on lines +16 to +17
.flat_map(|i| match&i.1 {
JuliaupConfigApplication::DevedApplication { path, julia_version, julia_depot, execution_aliases } => execution_aliases.iter().map(|j| (j.0.clone(), (j.1.target.clone(), path.clone(), julia_version.clone(), julia_depot.clone())))
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
.flat_map(|i| match&i.1 {
JuliaupConfigApplication::DevedApplication { path, julia_version, julia_depot, execution_aliases } => execution_aliases.iter().map(|j| (j.0.clone(), (j.1.target.clone(), path.clone(), julia_version.clone(), julia_depot.clone())))
.flat_map(|i| match &i.1 {
JuliaupConfigApplication::DevedApplication {
path,
julia_version,
julia_depot,
execution_aliases,
} => execution_aliases.iter().map(|j| {
(
j.0.clone(),
(
j.1.target.clone(),
path.clone(),
julia_version.clone(),
julia_depot.clone(),
),
)
}),
})
.map(|i| {
(
i.0.clone(),
(
i.1 .0.clone(),
i.1 .1.clone(),
i.1 .2.clone(),
i.1 .3.clone(),
),
)

.flat_map(|i| match&i.1 {
JuliaupConfigApplication::DevedApplication { path, julia_version, julia_depot, execution_aliases } => execution_aliases.iter().map(|j| (j.0.clone(), (j.1.target.clone(), path.clone(), julia_version.clone(), julia_depot.clone())))
})
.map(|i| (i.0.clone(), (i.1.0.clone(), i.1.1.clone(), i.1.2.clone(), i.1.3.clone())))
Copy link
Contributor

Choose a reason for hiding this comment

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

[rustfmt] reported by reviewdog 🐶

Suggested change
.map(|i| (i.0.clone(), (i.1.0.clone(), i.1.1.clone(), i.1.2.clone(), i.1.3.clone())))

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.

2 participants