feat: branch support in installation #224

royalpinto007 · 2023-12-31T05:08:48Z

Enables users to specify a branch during plugin installation in Coffee. This PR is a work in progress and not yet completed.

Reference- #205

netlify · 2023-12-31T05:08:54Z

✅ Deploy Preview for coffee-docs canceled.

Name	Link
🔨 Latest commit	`070e6d7`
🔍 Latest deploy log	https://app.netlify.com/sites/coffee-docs/deploys/65c36f4919cc910008bbc7e8

vincenzopalazzo

I do not see the code that does really this, but as a first set up code

coffee_cmd/src/cmd.rs

fix: arg macro

vincenzopalazzo

Some API upgrade is missing inside the test, make check should help to to upgrade

coffee_1  |    Compiling tests v0.1.0 (/workdir/tests)
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:115:10
coffee_1  |     |
coffee_1  | 115 |         .install("summary", true, true)
coffee_1  |     |          ^^^^^^^----------------------- an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 115 |         .install("summary", true, true, /* Option<std::string::String> */)
coffee_1  |     |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:170:35
coffee_1  |     |
coffee_1  | 170 |     let result = manager.coffee().install("summary", true, false).await;
coffee_1  |     |                                   ^^^^^^^------------------------ an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 170 |     let result = manager.coffee().install("summary", true, false, /* Option<std::string::String> */).await;
coffee_1  |     |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:176:10
coffee_1  |     |
coffee_1  | 176 |         .install("helpme", true, false)
coffee_1  |     |          ^^^^^^^----------------------- an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 176 |         .install("helpme", true, false, /* Option<std::string::String> */)
coffee_1  |     |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:279:35
coffee_1  |     |
coffee_1  | 279 |     let result = manager.coffee().install("summary", true, false).await;
coffee_1  |     |                                   ^^^^^^^------------------------ an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 279 |     let result = manager.coffee().install("summary", true, false, /* Option<std::string::String> */).await;
coffee_1  |     |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:288:35
coffee_1  |     |
coffee_1  | 288 |     let result = manager.coffee().install("x", true, false).await;
coffee_1  |     |                                   ^^^^^^^------------------ an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 288 |     let result = manager.coffee().install("x", true, false, /* Option<std::string::String> */).await;
coffee_1  |     |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:356:10
coffee_1  |     |
coffee_1  | 356 |         .install("summary", true, true)
coffee_1  |     |          ^^^^^^^----------------------- an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 356 |         .install("summary", true, true, /* Option<std::string::String> */)
coffee_1  |     |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:396:10
coffee_1  |     |
coffee_1  | 396 |         .install("summary", true, true)
coffee_1  |     |          ^^^^^^^----------------------- an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 396 |         .install("summary", true, true, /* Option<std::string::String> */)
coffee_1  |     |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:431:35
coffee_1  |     |
coffee_1  | 431 |     let result = manager.coffee().install("summary", true, false).await;
coffee_1  |     |                                   ^^^^^^^------------------------ an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 431 |     let result = manager.coffee().install("summary", true, false, /* Option<std::string::String> */).await;
coffee_1  |     |                                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:437:10
coffee_1  |     |
coffee_1  | 437 |         .install("helpme", true, false)
coffee_1  |     |          ^^^^^^^----------------------- an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 437 |         .install("helpme", true, false, /* Option<std::string::String> */)
coffee_1  |     |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | error[E0061]: this method takes 4 arguments but 3 arguments were supplied
coffee_1  |    --> tests/src/coffee_integration_tests.rs:496:10
coffee_1  |     |
coffee_1  | 496 |         .install("summary", true, false)
coffee_1  |     |          ^^^^^^^------------------------ an argument of type `Option<std::string::String>` is missing
coffee_1  |     |
coffee_1  | note: method defined here
coffee_1  |    --> /workdir/coffee_lib/src/plugin_manager.rs:14:14
coffee_1  |     |
coffee_1  | 14  |     async fn install(
coffee_1  |     |              ^^^^^^^
coffee_1  | help: provide the argument
coffee_1  |     |
coffee_1  | 496 |         .install("summary", true, false, /* Option<std::string::String> */)
coffee_1  |     |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
coffee_1  | 
coffee_1  | For more information about this error, try `rustc --explain E0061`.
coffee_1  | error: could not compile `tests` (lib test) due to 10 previous errors
coffee_1  | make: *** [Makefile:35: integration] Error 101

royalpinto007 · 2024-01-02T09:07:06Z

I don't see any issue with make check as it gives positive result, but integration testing fails.

For your reference-

royalpinto007@royalpinto007:~/coffee-dev$ make check
cargo test ""
    Finished test [unoptimized + debuginfo] target(s) in 1.46s
     Running unittests src/main.rs (target/debug/deps/coffee-a289af62a1ab4e5a)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/coffee_core-1e57aab876cba5b9)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/coffee_github-e12265faf851f5bf)

running 1 test
test tests::repository_is_initialized_ok ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 47.67s

     Running unittests src/lib.rs (target/debug/deps/coffee_httpd-9e02626a42513cf1)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/main.rs (target/debug/deps/coffee_httpd-7b9b9d4b60a3f65c)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/coffee_lib-e5e59cccfc42ee14)

running 3 tests
test plugin_conf::tests::test_remote ... ok
test url::tests::test_remote ... ok
test utils::tests::test_create_dir_in_home ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

     Running unittests src/main.rs (target/debug/deps/coffee_plugin-9a4709cab5419a40)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/coffee_storage-1e89ec4f4e859c18)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (target/debug/deps/coffee_testing-f5b11179a6057c76)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

vincenzopalazzo · 2024-01-02T09:13:20Z

Sorry It is my bad I give you the wrong information, this is the command that the CI runs https://github.com/coffee-tools/coffee/blob/master/docker/entrypoint.sh#L2

coffee_core/src/coffee.rs

tareknaser

Great progress! I've left some comments. Keep up the good work!

tareknaser · 2024-01-26T14:45:57Z

coffee_core/src/coffee.rs

@@ -247,6 +247,7 @@ impl PluginManager for CoffeeManager {
        plugin: &str,
        verbose: bool,
        try_dynamic: bool,
+        _branch: Option<String>,


This value isn't used anywhere

tareknaser · 2024-01-26T14:54:05Z

coffee_httpd/src/httpd/server.rs

@@ -91,7 +91,7 @@ async fn coffee_install(
    let try_dynamic = body.try_dynamic;

    let mut coffee = data.coffee.lock().await;
-    let result = coffee.install(plugin, false, try_dynamic).await;
+    let result = coffee.install(plugin, false, try_dynamic, None).await;


Idealy, you should also handle changing the httpd request to coffee.install() method
You can add the field to Install struct in coffee_lib/src/types/mod.rs

Maybe you meant this, but I am not sure if this is correct- d726c11

Maybe you meant this, but I am not sure if this is correct- d726c11

Correct, but why you are not passing the branch variable down the function call? I am really confused

It is easy, use the following code

Suggested change

let result = coffee.install(plugin, false, try_dynamic, None).await;

let branch = body.branch;

let result = coffee.install(plugin, false, try_dynamic, branch).await;

coffee_github/src/repository.rs

vincenzopalazzo · 2024-02-04T21:24:14Z

adding this to the milestone, but would love to have a summary of the status of this before starting review it

royalpinto007 · 2024-02-05T12:04:17Z

@vincenzopalazzo
I initially created a starter code for implementing the branch functionality. Upon @tareknaser's suggestions, I developed a switch_branch function, as shown in this commit: c664c66. After receiving some comments on it, I am currently at a standstill, as seen here: #224 (review).

vincenzopalazzo

Left some review, but the sentiments is the one of few months ago

looks like the real code it is missing :/ and this PR is just moving coding around

coffee_core/src/coffee.rs

coffee_github/src/repository.rs

vincenzopalazzo · 2024-02-06T19:13:32Z

coffee_httpd/src/httpd/server.rs

@@ -91,7 +91,7 @@ async fn coffee_install(
    let try_dynamic = body.try_dynamic;

    let mut coffee = data.coffee.lock().await;
-    let result = coffee.install(plugin, false, try_dynamic).await;
+    let result = coffee.install(plugin, false, try_dynamic, None).await;


Maybe you meant this, but I am not sure if this is correct- d726c11

Correct, but why you are not passing the branch variable down the function call? I am really confused

It is easy, use the following code

Suggested change

let result = coffee.install(plugin, false, try_dynamic, None).await;

let branch = body.branch;

let result = coffee.install(plugin, false, try_dynamic, branch).await;

vincenzopalazzo · 2024-02-06T19:14:43Z

coffee_lib/src/repository.rs

+    /// switch to the specified branch.
+    async fn switch_branch(&mut self, branch_name: &str) -> Result<(), CoffeeError>;


Suggested change

/// switch to the specified branch.

async fn switch_branch(&mut self, branch_name: &str) -> Result<(), CoffeeError>;

Does not maske sense have the switch_branch inside the repository interface, it can be just an util. For example look at the repository.upgrade code

coffee_plugin/src/plugin/plugin_mod.rs

vincenzopalazzo · 2024-02-06T19:16:44Z

tests/src/coffee_integration_tests.rs

-        .install("summary", true, false)
+        .install("summary", true, false, None)


Try to write a test also where you specify the branch.

…fee-dev into branch-install

vincenzopalazzo requested changes Dec 31, 2023

View reviewed changes

coffee_cmd/src/cmd.rs Outdated Show resolved Hide resolved

feat: branch support in installation

cf1703b

fix: arg macro

royalpinto007 force-pushed the branch-install branch from 349dbb9 to cf1703b Compare January 1, 2024 09:16

vincenzopalazzo reviewed Jan 2, 2024

View reviewed changes

vincenzopalazzo reviewed Jan 3, 2024

View reviewed changes

coffee_core/src/coffee.rs Outdated Show resolved Hide resolved

fix: integration_tests bugs

94095f1

royalpinto007 marked this pull request as draft January 6, 2024 16:28

feat: switch_branch func

c664c66

royalpinto007 marked this pull request as ready for review January 26, 2024 12:37

tareknaser suggested changes Jan 26, 2024

View reviewed changes

royalpinto007 force-pushed the branch-install branch from cda659d to c664c66 Compare January 26, 2024 16:20

royalpinto007 added 3 commits January 26, 2024 22:40

feat: keep track of current branch

06f5f4f

feat: branch in install struct

d726c11

fix: integration ci

fce999b

vincenzopalazzo added this to the 0.1-rc1 milestone Feb 4, 2024

vincenzopalazzo requested changes Feb 6, 2024

View reviewed changes

royalpinto007 added 9 commits February 7, 2024 15:12

fix: use branch

a2fab1e

resolve: merge conflicts

bd718b9

fix: lint ci

ddffe9f

fix: merge conflicts

d9a5d64

fix: integration_tests bugs

54314f6

feat: switch_branch func

2f47af7

feat: keep track of current branch

1954db7

feat: branch in install struct

52e1b99

fix: integration ci

5863482

royalpinto007 added 3 commits February 7, 2024 17:07

fix: merge conflicts

ba4a315

fix: ci errors

61f569a

Merge branch 'branch-install' of https://github.com/royalpinto007/cof…

070e6d7

…fee-dev into branch-install

royalpinto007 closed this Feb 7, 2024

royalpinto007 deleted the branch-install branch February 7, 2024 12:12

royalpinto007 mentioned this pull request Feb 7, 2024

feat: branch installation #235

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: branch support in installation #224

feat: branch support in installation #224

royalpinto007 commented Dec 31, 2023

netlify bot commented Dec 31, 2023 •

edited

Loading

vincenzopalazzo left a comment

vincenzopalazzo left a comment

royalpinto007 commented Jan 2, 2024

vincenzopalazzo commented Jan 2, 2024

tareknaser left a comment

tareknaser Jan 26, 2024

tareknaser Jan 26, 2024

royalpinto007 Jan 27, 2024

vincenzopalazzo Feb 6, 2024

vincenzopalazzo commented Feb 4, 2024

royalpinto007 commented Feb 5, 2024

vincenzopalazzo left a comment

vincenzopalazzo Feb 6, 2024

vincenzopalazzo Feb 6, 2024

vincenzopalazzo Feb 6, 2024

	let result = coffee.install(plugin, false, try_dynamic, None).await;
	let branch = body.branch;
	let result = coffee.install(plugin, false, try_dynamic, branch).await;

		/// switch to the specified branch.
		async fn switch_branch(&mut self, branch_name: &str) -> Result<(), CoffeeError>;

		.install("summary", true, false)
		.install("summary", true, false, None)

feat: branch support in installation #224

feat: branch support in installation #224

Conversation

royalpinto007 commented Dec 31, 2023

netlify bot commented Dec 31, 2023 • edited Loading

✅ Deploy Preview for coffee-docs canceled.

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo left a comment

Choose a reason for hiding this comment

royalpinto007 commented Jan 2, 2024

vincenzopalazzo commented Jan 2, 2024

tareknaser left a comment

Choose a reason for hiding this comment

tareknaser Jan 26, 2024

Choose a reason for hiding this comment

tareknaser Jan 26, 2024

Choose a reason for hiding this comment

royalpinto007 Jan 27, 2024

Choose a reason for hiding this comment

vincenzopalazzo Feb 6, 2024

Choose a reason for hiding this comment

vincenzopalazzo commented Feb 4, 2024

royalpinto007 commented Feb 5, 2024

vincenzopalazzo left a comment

Choose a reason for hiding this comment

vincenzopalazzo Feb 6, 2024

Choose a reason for hiding this comment

vincenzopalazzo Feb 6, 2024

Choose a reason for hiding this comment

vincenzopalazzo Feb 6, 2024

Choose a reason for hiding this comment

netlify bot commented Dec 31, 2023 •

edited

Loading