Open-CMSIS-Pack / cpackget

Open-CMSIS-Pack Package Installer
Apache License 2.0
23 stars 14 forks source link

[cpackget] Define the behavior of cpackget if the CMSIS-Pack root directory is "incomplete" #137

Open jkrech opened 1 year ago

jkrech commented 1 year ago

Describe The Problem To Be Solved I would like cpackget to be more robust or fault tolerant under different circumstances where the CMSIS-Pack repository directory does not (yet) contain expected directories and files. However I think there is a fine line of trying to be too clever, potentially covering up or hiding issues in the user's setup.

.Download/
.Local/
.Local/local_repository.pidx
.Web/
.Web/index.pidx

Scenarios: a) assume that for some reason the .Local/ directory does not exist. Is that acceptable? Would it be okay for cpackget to silently create it? Would it create an empty local_repositories.pidx silently as well? Would cpackget only create it in commands where it updates local_repositories.pidx or adds files to the directory but not complain about it not existing?

b) assume that for some reason the .Web/ directory does not exist or it exists and is empty. Is that acceptable? Would it be okay for cpackget to silently create the directory and add an empty index.pidx? Or would cpackget fail only for commands that strictly require the content of this directory and pidx file?

c) assume that for some reason the .Download/ directory does not exist. Is that acceptable? Would tools simply create it when it runs a command that ought to store a file in the directory?

d) assume the use accidentally misconfigured the CMSIS-Pack root directory. Should the tool complain that there is no files in that directory? Should the tool create the directory if it did not exist?

lud0v1c commented 1 year ago

I agree that there should be some default behavior regarding the consistency of the pack root, and it shouldn't vary from command to command. Would it not be best, on all commands that involve reading/writing the pack root, to perform the same checks and recovery processes? Ideally we want every user to have the same CMSIS-Pack repository organization, and by performing a healtcheck/recovery after executing a command, we could guarantee this.

madchutney commented 1 year ago

I think we should consider a separation of concerns and focus on the different tasks that are being undertaken. For example, the issue that has triggerred this ticket to be raised is not caused by a direct invocation of cpackget. The software is attempting to build a cprj using cbuild, which has had the side effect of attempting to create a .Local.

When cpackget is invoked to download packs it would seem reasonable that it would create the necessary directories to perform the operation. For example creating .Web/, downloading .Web/index.pidx, creating .Download/ if being asked to store the pack files and actually unarchiving any downloaded packs.

It wouldn't seem necessary to create .Local/ or .Local/local_repository.pidx in order to install packs. Although that may a different operation handled by cpackget.

Likewise for cbuild its purpose should be to perform a build. It would seem reasonable that it generates errors should a required pack not be available. But as far as I understand the build operation does not require .Web/ and would only require .Local/ if local packs have been defined. (and up until very recently this was the case). this is still the case GH.

lud0v1c commented 1 year ago
That does make sense. Probably best for you guys to define such behavior, as I'm not a user of cbuild or the the rest of the c-family. Perhaps a good first step would be to define the inputs/outputs of each command? Something like this: command .Web .Local .Download called by cbuild
add R R/W R/W Yes
init W W W Yes
... ... ... ... ...

(just an example, might not be accurate)

jkrech commented 1 year ago

@madchutney the invocation of cbuild myproject.cprj ... --packs has been invoking cpackget pack add which reported the error message:

E: Directory(ies) "...../.Local" are missing! Was ..../ initialized correctly?

Note if you had not specified --pack above cpackget would have never been called. The purpose of cbuild is orchestration! Whether or not you think that fetching resources is part of what cbuild shall orchestrate or not is a different debate. We found the compromise by adding the command line option --packs to vary the scope of cbuild.

madchutney commented 1 year ago

@jkrech Thanks, for the case I mentioned we can use cbuild without the --packs option. That seems a reasonable compromise to me, it just caught us out this time round. I wasn't aware that cbuild was considered an orchestrator, is it the intention that it will also perform orchestration with other tools in the future, such as csolution?

I like @lud0v1c suggestion of defining which operations require access to each of the resources. This would be nice to do for each of the tools available, it would help understand some assumptions around how the tools should be used.

In the general case I believe tools should only return and error or fail if the resources they require or operate on are not available. Hypothetically, if a project didn't need any packs we could argue that there was no need for the CMSIS Pack repository directory at all. I think this is taking it a little far as these tools are for the CMSIS ecosystem, but I hope it illustrates my point. The flip side is that we have to be careful that we don't get obscure errors because the resources aren't available.

lud0v1c commented 1 year ago

@jkrech what do you think would be the right procedure here? As I said, I'm not that familiar with the rest of the devtools/c-family, and that's why I advocated for a general approach and behaviour to the scenarios you described.

jkrech commented 1 year ago

@thorstendb-ARM could you create a table similar to the one suggested above, which details which commands of cpackget access which folder of the $CMSIS_PACK_ROOT folder (read, write) and what cpackget shall do in case this folder does not currently exist. My assumption is that if cpackget writes to a location that does not exist, that it will automatically create it. In case of read, I would assume that the folder could be ignored and/or a warning message is displayed that a folder and the corresponding information is not present.