fuelup icon indicating copy to clipboard operation
fuelup copied to clipboard

feat: support toolchain export

Open Halimao opened this issue 1 year ago • 12 comments

close #322

Support fuelup toolchain export

image

image

image

image

Halimao avatar Nov 23 '23 15:11 Halimao

@sdankel GM sir, looking for your review again😀

Halimao avatar Dec 01 '23 02:12 Halimao

@sdankel Hi sir, looking forward for your review again😀

Halimao avatar Dec 16 '23 01:12 Halimao

Question about the exporting format in the images. It looks to be Json-ish.

Here is an example of a toolchain file and they look to be formatted differently.

Am I overlooking something or are the formats different when exported?

Exporting format is standard toml format as well, you can check it here: https://www.toml-lint.com/ . image

Halimao avatar Dec 20 '23 10:12 Halimao

@Braqzen @sdankel Hi sir, is there anything that needs to be added for this pr?

Halimao avatar Jan 05 '24 03:01 Halimao

Running toolchain export without any args / options I see the following

1

I would move the message asking for input onto a new line instead of having a single long line. The default is not automatically exported, it prompts for input.

The invalid channel message comes from a break in backwards compatibility. I haven't followed the changes to know how we manage new toolchains etc. so I'm unsure if this x86 channel is valid but I have updated fuelup and installed beta-4 so I assume that it's still valid. All that to say, we are forcing a new format, have we considered the current / old format and whether we ought to support that?

Running toolchain export with beta-4 I get a file containing the toolchain and components however when I run latest-YYYY-MM-DD I get the following error

1

We ought to handle the difference between a valid channel that the user can type in ex. the beta-X vs the example formats which include a date (as seen above).

According to the possible argument values I should be able to use latest however when I do that it tells me that it's an invalid channel.

1

Overriding an existing toolchain uses the incorrect toolchain. It ignores my input and uses the default now? When I check the content of the file it has beta-4 so it only prints the incorrect toolchain.

1

I think this should be discussed with @sdankel first. According to the confusion listed above, it seems that you have mixed up the toolchain name and toolchain channel.


Such as

Running toolchain export without any args / options shows the promotions that ask for a valid channel.

Reference to the comments by @sdankel This is caused by the default toolchain name you have(latest-x86_64_unknown...) is not a valid channel but a valid toolchain name. When there is no --channel provided, we will use the default toolchain name as the channel, and then check the validation of channel


According to the possible argument values I should be able to use latest however when I do that it tells me that it's an invalid channel.

Just as the help message described, latest is a possible argument value for toolchain name, not the toolchain channel. But for the beta-4, it's not only a valid toolchain name, but also a valid toolchain channel

Halimao avatar Jan 11 '24 03:01 Halimao

I would also split the tests a bit further to group them in order to reduce the cognitive load and make it easier to read.

I would opt for either putting all the success cases at the top and panics at the bottom or consider putting the success cases in a mod success and the panics in a mod panic.

After the tests are split I'll look over test coverage because I haven't exactly checked if all cases are covered

Addressed!

Halimao avatar Jan 19 '24 10:01 Halimao

This produces a file that looks like this:

toolchain = { channel = "beta-3" }
components = { forc = "0.46.1", forc-explore = "0.28.1", forc-tx = "0.46.1", forc-deploy = "0.46.1", forc-wallet = "0.3.0", fuel-core = "0.20.5", forc-run = "0.46.1" }

But we want it formatted like this:

[toolchain]
channel = "beta-3"
    
[components]
forc-wallet = "0.3.0",
forc-deploy = "0.46.1",
forc-run = "0.46.1",
forc-explore = "0.28.1",
forc-tx = "0.46.1",
forc = "0.46.1",
fuel-core = "0.20.5"

sdankel avatar Jan 29 '24 22:01 sdankel

I fixed the formatting issue and added tests for it. However I noticed it still pulls the wrong name for the channel by default. When I'm on beta-3, it pulls the full toolchain name rather than the shortened dist name for the channel.

This test should pass:

    #[test]
    #[serial]
    fn test_export_beta3_full() {
        create_settings_file();
        create_toolchain_info();

        export(
            ExportCommand {
                name: None,
                channel: Some("beta-3-x86_64-apple-darwin".to_string()),
                force: true,
            },
            &INPUT_NO[..],
        )
        .expect("should succeed");
        assert_toolchain_info(BETA_3_FILE_CONTENTS);
    }

sdankel avatar Jan 30 '24 02:01 sdankel

This test should pass:

            ExportCommand {
                name: None,
                channel: Some("beta-3-x86_64-apple-darwin".to_string()),
                force: true,
            },
            &INPUT_NO[..],
        )

One of the valid channel should be <latest-YYYY-MM-DD|nightly-YYYY-MM-DD|beta-1|beta-2|beta-3|beta-4>, not sure if this("beta-3-x86_64-apple-darwin" is not a valid channel) is the reason why this test didn't pass?

Halimao avatar Jan 30 '24 06:01 Halimao

This test should pass:

            ExportCommand {
                name: None,
                channel: Some("beta-3-x86_64-apple-darwin".to_string()),
                force: true,
            },
            &INPUT_NO[..],
        )

One of the valid channel should be <latest-YYYY-MM-DD|nightly-YYYY-MM-DD|beta-1|beta-2|beta-3|beta-4>, not sure if this("beta-3-x86_64-apple-darwin" is not a valid channel) is the reason why this test didn't pass?

What I'm saying is if I'm on beta-3, the toolchain name is beta-3-x86_64-apple-darwin. If I just run fuelup toolchain export, I expect it to work. Currently it gives me the error that beta-3-x86_64-apple-darwin is not a valid channel. I think this is because of https://github.com/FuelLabs/fuelup/issues/545

sdankel avatar Jan 31 '24 00:01 sdankel

What I'm saying is if I'm on beta-3, the toolchain name is beta-3-x86_64-apple-darwin. If I just run fuelup toolchain export, I expect it to work. Currently it gives me the error that beta-3-x86_64-apple-darwin is not a valid channel. I think this is because of https://github.com/FuelLabs/fuelup/issues/545

I'm sorry, but this is expected IMO. As we discussed before, reference to this thread

  1. if no toolchain name is provided(run only fuelup toolchain export), we will use the default toolchain name, in your local env, that's beta-3-x86_64-apple-darwin.
  2. Also, as there is no --channel provided, we will try to use the toolchain name as channel first.
  3. Then check the channel name, it will prompt user "beta-3-x86_64-apple-darwin is not a valid channel, please input a valid one"
  4. If user input beta-3, then this will success as beta-3 is a valid channel, otherwise, abort the process as input channel is invalid
image

Halimao avatar Jan 31 '24 02:01 Halimao

if no toolchain name is provided(run only fuelup toolchain export), we will use the default toolchain name, in your local env, that's beta-3-x86_64-apple-darwin.

Yes, and we should parse out beta-3 from the toolchain name using DistToolchainDescription::from_str once https://github.com/FuelLabs/fuelup/pull/566 lands.

sdankel avatar Feb 02 '24 23:02 sdankel