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

Projects in the west directory are not forbidden #102

Closed
mbolivar opened this issue Nov 8, 2018 · 7 comments · Fixed by #754
Closed

Projects in the west directory are not forbidden #102

mbolivar opened this issue Nov 8, 2018 · 7 comments · Fixed by #754
Assignees
Labels
enhancement New feature or request priority: low
Milestone

Comments

@mbolivar
Copy link
Contributor

mbolivar commented Nov 8, 2018

Originally reported by @tejlmand.

For example, even with forbidding projects named west .west or manifest, this manifest fragment is legal:

  projects:
    - name: not-west
      path: .west

This creates a structure where the top-level west directory contains a .git for the not-west repository, which is very strange.

Arguably this is user error but the current behavior seems undesirable. We should consider what to do here:

  • warn
  • error
  • skip the project?
  • ...
@mbolivar mbolivar added priority: low bug Something isn't working labels Nov 8, 2018
@tejlmand
Copy link
Collaborator

I would suggest to error out.

And in future we could consider to add --force to west to allow user to overrule whatever restriction west poses

@tejlmand tejlmand self-assigned this Nov 20, 2018
@mbolivar
Copy link
Contributor Author

And in future we could consider to add --force to west to allow user to overrule whatever restriction west poses

This would require adding --force to basically every invocation of west that touches a project or parses a manifest.

Perhaps a manifest configuration option?

@tejlmand
Copy link
Collaborator

@mbolivar you're right, it would be required for every command, expect maybe west list ;)

I think my main point was to suggest the error for now, and then take it up, if someone needs this.

@mbolivar
Copy link
Contributor Author

Erroring out seems sane.

tejlmand added a commit to tejlmand/west that referenced this issue Nov 29, 2018
Fixes: zephyrproject-rtos#102

This commit introduces a list of restricted folders that are compared
against project defined paths to ensure a manifest does not contain
paths ending in the west folder, e.g.:
- west
- west/foo
- foo/../west

while still allowing folders, such as:
- west-ext
as project folder.

Signed-off-by: Torsten Rasmussen <torsten.rasmussen@nordicsemi.no>
tejlmand added a commit to tejlmand/west that referenced this issue Dec 3, 2018
Fixes: zephyrproject-rtos#102

This commit introduces a list of restricted folders that are compared
against project defined paths to ensure a manifest does not contain
paths ending in the west folder, e.g.:
- west
- west/foo
- foo/../west

while still allowing folders, such as:
- west-ext
as project folder.

Signed-off-by: Torsten Rasmussen <torsten.rasmussen@nordicsemi.no>
@mbolivar
Copy link
Contributor Author

Migrating to enhancement

@mbolivar mbolivar added enhancement New feature or request and removed bug Something isn't working labels May 15, 2019
@marc-hb
Copy link
Collaborator

marc-hb commented Oct 11, 2024

Since this issue was filed, WEST_DIR has been renamed to hidden directory .west by commit 045b591 and has changed A LOT in nature.

BUT I tested this again with path: .west/foo and with (even worse!) path: .west and west version 1.3 still did not mind. So this issue is still valid.

@marc-hb
Copy link
Collaborator

marc-hb commented Oct 15, 2024

Fix submitted by @pdgendt in #754

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