Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jkrech opened this issue Nov 18, 2022 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@jkrech
Copy link
Member

jkrech commented Nov 18, 2022

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?

@jkrech jkrech added the enhancement New feature or request label Nov 18, 2022
@lud0v1c
Copy link
Contributor

lud0v1c commented Nov 18, 2022

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
Copy link

madchutney commented Nov 18, 2022

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
Copy link
Contributor

lud0v1c commented Nov 18, 2022

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
Copy link
Member Author

jkrech commented Nov 18, 2022

@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.

@lud0v1c lud0v1c assigned lud0v1c and unassigned lud0v1c Nov 18, 2022
@madchutney
Copy link

madchutney commented Nov 18, 2022

@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
Copy link
Contributor

lud0v1c commented Nov 23, 2022

@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 jkrech assigned jkrech and unassigned lud0v1c Mar 7, 2023
@jkrech
Copy link
Member Author

jkrech commented Jul 4, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants