feat: Allow install, list and use from commit #2659

Arrowana · 2023-10-12T01:03:53Z

avm list

0.23.0
0.24.0
0.24.1
0.24.2
0.25.0
0.26.0  (installed)
0.27.0  (installed)
0.28.0  (latest, installed)
0.28.0-dcafb789e1635b8f90e8fd3badd0f9a015d204d4 (installed, current)

Questions:

Full commit or shorter? I think full commit is fine to avoid having to do even more guessing in the cli
I introduce the is_commit flag, because otherwise we don't know if it is a version or a commit that want to be installed. We could parse the version, if it isn't one we query the commit for the string if none of those 2 Ok we return Not a valid version or commit. What do you think? Single arg trying ver then commit, less clunky than the extra flag
Once installed anchor --version gives the last release for the commit, well i don't think we can fix this without pulling the repo and changing Cargo.toml

vercel · 2023-10-12T01:03:57Z

@Arrowana is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

acheroncrypto

Full commit or shorter? I think full commit is fine to avoid having to do even more guessing in the cli

Don't have strong opinions but would be nice to also support shorter commit hashes.

Once installed anchor --version gives the last release for the commit, well i don't think we can fix this without pulling the repo and changing Cargo.toml

I think it's fine as long as the commit can be viewed from avm.

acheroncrypto · 2023-10-12T13:03:20Z

avm/src/main.rs

+        /// Anchor version or commit
+        version: String,
+        #[clap(short, long, required = false)]
+        is_commit: bool,


Single arg trying ver then commit, less clunky than the extra flag

I'm guessing this is not implemented yet. Don't think is_commit flag is necessary, can just decide whether it's a version or a commit based on the input.

Forgot to push, pushed and back to single arg

Trying to install an incorrect commit

target/debug/avm install dcafb789e1635b8f90e8fd3badd0f9a015d204d42ds2` error: invalid value 'dcafb789e1635b8f90e8fd3badd0f9a015d204d42ds2' for '<VERSION_OR_COMMIT>': Not a valid version or commit: unexpected character 'd' while parsing major version number, Error checking commit dcafb789e1635b8f90e8fd3badd0f9a015d204d42ds2: {"message":"No commit found for SHA: dcafb789e1635b8f90e8fd3badd0f9a015d204d42ds2","documentation_url":"https://docs.github.com/rest/commits/commits#get-a-commit"}

We can also provide a shortened commit sha with
518b390

acheroncrypto · 2023-10-13T09:15:52Z

avm/src/main.rs

+                })
+        })
+}
+
 pub fn entry(opts: Cli) -> Result<()> {
    match opts.command {
        Commands::Use { version } => avm::use_version(version),


I've tried it and confirm it works as expected but one area that might need improvement is avm use command as it requires putting the full version-hash avm use 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 and doesn't work with avm use 6cf200.

acheroncrypto

It'd be great to make it work with #2649. Specifying:

[toolchain]
anchor_version = "0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23"

only works if that version is already installed because avm install 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 doesn't work.

Can implement this in another PR so I'm merging this as is, thanks!

Arrowana · 2023-10-15T03:59:40Z

It'd be great to make it work with #2649. Specifying:
[toolchain]
anchor_version = "0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23"
only works if that version is already installed because avm install 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 doesn't work.

Can implement this in another PR so I'm merging this as is, thanks!

Since we install from the commit alone should it be?

[toolchain]
anchor_version = "6cf200493a307c01487c7b492b4893e0d6f6cb23"

Because if you think about it if we allow 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 it will install 6cf200493a307c01487c7b492b4893e0d6f6cb23 but then there is no guarantee it is based on 0.28.0, or we could validate this if someone does avm install 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23

acheroncrypto · 2023-10-15T09:17:17Z

It'd be great to make it work with #2649. Specifying:
[toolchain]
anchor_version = "0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23"
only works if that version is already installed because avm install 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 doesn't work.
Can implement this in another PR so I'm merging this as is, thanks!
Since we install from the commit alone should it be?
[toolchain]
anchor_version = "6cf200493a307c01487c7b492b4893e0d6f6cb23"

That's what I thought at first but later realized version-commit is a bit more explicit and also it doesn't require any changes to how the toolchain override works because we just use the Anchor binary name which is 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23.

Because if you think about it if we allow 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 it will install 6cf200493a307c01487c7b492b4893e0d6f6cb23 but then there is no guarantee it is based on 0.28.0, or we could validate this if someone does avm install 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23

We could validate the version for the commit is correct here but I don't think it's necessary because even if someone puts an incorrect version for the commit

[toolchain]
anchor_version = "0.27.0-6cf200493a307c01487c7b492b4893e0d6f6cb23"

running an anchor command outputs

`anchor` 0.27.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 is not installed with `avm`. Installing...

Version 0.28.0-6cf200493a307c01487c7b492b4893e0d6f6cb23 is already installed
Error: No such file or directory (os error 2)

so it's pretty clear what's wrong here.

feat: Allow install, list and use from commit

99e55b4

parse install target instead

eceaa76

acheroncrypto reviewed Oct 12, 2023

View reviewed changes

Arrowana added 2 commits October 13, 2023 08:32

Support shortened commit sha as input

518b390

clippy lint

4cec0b4

acheroncrypto added the avm label Oct 13, 2023

acheroncrypto reviewed Oct 13, 2023

View reviewed changes

acheroncrypto approved these changes Oct 14, 2023

View reviewed changes

acheroncrypto merged commit 5900c93 into coral-xyz:master Oct 14, 2023
47 of 48 checks passed

acheroncrypto mentioned this pull request Oct 14, 2023

avm: Install from version-commit and refactor #2671

Merged

Arrowana deleted the feat/allow-install-list-and-use-from-commit branch October 15, 2023 03:56

acheroncrypto added a commit to acheroncrypto/anchor that referenced this pull request Oct 15, 2023

Add missing CHANGELOG entry for coral-xyz#2659

c23b2ea

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Allow install, list and use from commit #2659

feat: Allow install, list and use from commit #2659

Arrowana commented Oct 12, 2023 •

edited

Loading

vercel bot commented Oct 12, 2023

acheroncrypto left a comment

acheroncrypto Oct 12, 2023

Arrowana Oct 12, 2023 •

edited

Loading

acheroncrypto Oct 13, 2023

acheroncrypto left a comment

Arrowana commented Oct 15, 2023

acheroncrypto commented Oct 15, 2023

feat: Allow install, list and use from commit #2659

feat: Allow install, list and use from commit #2659

Conversation

Arrowana commented Oct 12, 2023 • edited Loading

vercel bot commented Oct 12, 2023

acheroncrypto left a comment

Choose a reason for hiding this comment

acheroncrypto Oct 12, 2023

Choose a reason for hiding this comment

Arrowana Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

acheroncrypto Oct 13, 2023

Choose a reason for hiding this comment

acheroncrypto left a comment

Choose a reason for hiding this comment

Arrowana commented Oct 15, 2023

acheroncrypto commented Oct 15, 2023

Arrowana commented Oct 12, 2023 •

edited

Loading

Arrowana Oct 12, 2023 •

edited

Loading