add: CLI option to specify config path

[Nukesor]

Hi!

This PR adds the --config option to the install, uninstall and update subcommand.
(Not sure about the parameter name. I went with --config, as the file is located in the ~/.config folder)

The use-case for this feature is caching on Gitlab CI runners.
The Gitlab CI only allows caching inside the project folder.
Since the project is always a subdirectory of the gitlab runner's user home directory, it's impossible to cache files that're located in the home directory or in our case the ~/.config directory.

Without the ~/.config/espup/espup.toml, espup update runs simply fail.

A possible workaround for this would be #61, but that would be quite a bit more work and also feels a bit hacky.

[SergioGasquez]

Hi! Thanks for your contribution!
Aside from the few small comments that I left, it may make sense to store config_path in the Config struct (maybe even create a constructor that takes it as an input argument) that way we don't need to pass the config_path argument to all the Config methods.

[Nukesor]

Should I ignore the config field on the Config struct, when serializing and deserializing?

[Nukesor]

Thanks for the review and your work on this project :)

The path is now contained inside the Config.

It feels a bit hacky to keep a "temporary" field in the Config struct, but I see how this is convenient from a developer perspective.

[SergioGasquez]

Sorry, I didn't notice this small thing before. Also, it should be a good moment to update the log message when we create the config file including the path:

    info!("{} Saving configuration file at '{}'", emoji::WRENCH, config_path);

I agree with your opinion, not sure if it's the best idea to have the path stored in the Config struct. What about having a ConfigFile struct that stores a Config and config_path? We could make a constructor for it, and probably move the get_config_path method to it.

Sorry for requesting so many changes, I am happy to help if you want!

[Nukesor]

Sorry, I didn't notice this small thing before. Also, it should be a good moment to update the log message when we create the config file including the path:

    info!("{} Saving configuration file at '{}'", emoji::WRENCH, config_path);

Done

I agree with your opinion, not sure if it's the best idea to have the path stored in the Config struct. What about having a ConfigFile struct that stores a Config and config_path? We could make a constructor for it, and probably move the get_config_path method to it.

The datastructure is definitely cleaner, but this also introduced quite a bit of code.
I didn't extract get_config_path into the constructor, as we also need it in the load function.

Sorry for requesting so many changes, I am happy to help if you want!

Not a problem :)

Just tell me if you're happy with those changes or if I should revert them^^

[SergioGasquez]

The datastructure is definitely cleaner, but this also introduced quite a bit of code. I didn't extract get_config_path into the constructor, as we also need it in the load function.

I think we are in a good spot to merge it! Maybe we can revisit it in the future to improve it if we find a better way to handle it. Just did a small pr to your fork: Nukesor#1 with some minor formatting changes.

Thanks for all your work!

[Nukesor]

Awesome, thanks!

If it isn't too much of a problem, It would be awesome if you could also push a new release :)
This feature would make life a lot easier for us 😆

[SergioGasquez]

If it isn't too much of a problem, It would be awesome if you could also push a new release :) This feature would make life a lot easier for us laughing

Yes, we will like to make a new release as soon as we merge https://github.com/esp-rs/espup/tree/feature/github-token. As it includes a few changes that will be required for our next xtensa-toolchain release. I would like to have it done by tomorrow, but lets see!

[SergioGasquez]

Hi @Nukesor!

Sorry for taking so long to do the new release. It's now published!