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

app: project: Allow updating only cloned projects #781

Closed
wants to merge 2 commits into from

Conversation

pdgendt
Copy link
Collaborator

@pdgendt pdgendt commented Feb 6, 2025

Add an update command argument and configuration option to allow updating the currently cloned projects.

This can help in the scenario where a user updated an initial set of projects manually and would like to only update those in the future.

Add an update command argument and configuration option to allow
updating the currently cloned projects.

This can help in the scenario where a user updated an initial set of
projects manually and would like to only update those in the future.

Signed-off-by: Pieter De Gendt <pieter.degendt@basalte.be>
Add a test case for passing --cloned to west update or as a
configuration option.

Signed-off-by: Pieter De Gendt <pieter.degendt@basalte.be>
@pdgendt pdgendt marked this pull request as ready for review February 6, 2025 20:11
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biggest source of doubt I have about this comes from how it should work when you have a project that is:

  1. not cloned
  2. contains an import: in the manifest

The behavior here can be pretty counter-intuitive and confusing -- did you consider this in detail? What's the plan here?

For a real world example, consider this top level west.yml fragment:

manifest:
  projects:
    - name: p1
      import: true
    - name: p2
      import: true

then p1/west.yml says:

manifest:
  projects:
    - name: foo
      url: url1

while p2/west.yml says:

manifest:
  projects:
    - name: foo
      url: url2

If you do west update --cloned with p1 and p2 both cloned, you get foo from url1. But if you do west update --cloned with just p2 cloned, you get foo from url2.

I find this sort of behavior pretty confusing and I'm not sure what the "right" thing is in terms of what the user would reasonably expect:

  1. should we just fail if any project with an import is not cloned?
  2. should we ignore it and therefore allow us to get imported projects from surprising places (url2 instead of url1)?
  3. something else?

Comment on lines +956 to +957
help='''update the projects that are already cloned
in the current workspace''')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since update_some() is being changed to match, I think the meaning of this option is closer to "only update a project if it's already cloned locally" or so, right?

I'm thinking about the behavior of west update --cloned foo for an uncloned project foo and feeling the current help doesn't make clear how it should work in this case.

@pdgendt
Copy link
Collaborator Author

pdgendt commented Feb 6, 2025

The biggest source of doubt I have about this comes from how it should work when you have a project that is:

  1. not cloned
  2. contains an import: in the manifest

The behavior here can be pretty counter-intuitive and confusing -- did you consider this in detail? What's the plan here?

If you do west update --cloned with p1 and p2 both cloned, you get foo from url1. But if you do west update --cloned with just p2 cloned, you get foo from url2.

I see, but how is this different from calling west update p2 without having cloned p1? The idea was to give users the option of working in/with the upstream Zephyr repository, and having only a subset of HALs/tools/... cloned, without the need to remember which were already updated.

An "end-goal" I envisioned is to have a West extension in Zephyr, for example west getting-started, that would present a cli wizard to the user to install a bare minimum setup (HAL + toolchain + ..) instead of cloning/installing everything. This option could be useful in such a setup IMO.

I find this sort of behavior pretty confusing and I'm not sure what the "right" thing is in terms of what the user would reasonably expect:

  1. should we just fail if any project with an import is not cloned?
  2. should we ignore it and therefore allow us to get imported projects from surprising places (url2 instead of url1)?
  3. something else?

I tried to keep it as simple as possible, but if it would cause more confusion than it could be useful, I'm happy to drop it.

@pdgendt
Copy link
Collaborator Author

pdgendt commented Feb 6, 2025

The biggest source of doubt I have about this comes from how it should work when you have a project that is:

  1. not cloned
  2. contains an import: in the manifest

The behavior here can be pretty counter-intuitive and confusing -- did you consider this in detail? What's the plan here?
If you do west update --cloned with p1 and p2 both cloned, you get foo from url1. But if you do west update --cloned with just p2 cloned, you get foo from url2.

I see, but how is this different from calling west update p2 without having cloned p1? The idea was to give users the option of working in/with the upstream Zephyr repository, and having only a subset of HALs/tools/... cloned, without the need to remember which were already updated.

Hmn, it does differ as only p2 would be updated and foo wouldn't. The devil is in the details.

An "end-goal" I envisioned is to have a West extension in Zephyr, for example west getting-started, that would present a cli wizard to the user to install a bare minimum setup (HAL + toolchain + ..) instead of cloning/installing everything. This option could be useful in such a setup IMO.

Maybe this would be more suited for the example-application repository or even a dedicated one.

I find this sort of behavior pretty confusing and I'm not sure what the "right" thing is in terms of what the user would reasonably expect:

  1. should we just fail if any project with an import is not cloned?
  2. should we ignore it and therefore allow us to get imported projects from surprising places (url2 instead of url1)?
  3. something else?

I tried to keep it as simple as possible, but if it would cause more confusion than it could be useful, I'm happy to drop it.

I'll close this for now :) thanks @mbolivar for your time and sorry for the noise.

@pdgendt pdgendt closed this Feb 6, 2025
@marc-hb
Copy link
Collaborator

marc-hb commented Feb 6, 2025

Just for the record, this looked like an attempt to implement:

I find this sort of behavior pretty confusing and I'm not sure what the "right" thing is in terms of what the user would reasonably expect:

Sounds like we just got yet another PR/issue with the infamous label Partial imports Incomplete or changing imports are much more complicated than you think

@marc-hb marc-hb added the Partial imports Incomplete or changing imports are much more complicated than you think label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Partial imports Incomplete or changing imports are much more complicated than you think
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants