`west build` might destroy your repository, as it is defaulting doing pristine.

[tejlmand]

If a user accidentally runs cmake -DBOARD=<board> path/to/sample at the root of a repository, several files will be created. The user of course has the responsibility cleanup this mess.

However, in case the user calls west build after such a mistake, west will destroy the users complete repository. And this must not happen.

To reproduce: (WARNING This procedure will wipe your repo.)

$ west init -m https://github.com/nrfconnect/sdk-nrf
$ west update
$ cd nrf
$ cmake -GNinja -DBOARD=nrf52840dk_nrf52840 samples/bluetooth/peripheral_bms/
# Failed, user doesn't clean up

# 2 months later, now using west
$ west build -b nrf52840dk_nrf52840 samples/bluetooth/peripheral_lbs/
-- west build: making build dir /tmp/ncs/nrf pristine   # <--- west doing a pristine at my toplevel, not in a build folder :-(
-- west build: generating a build system
CMake Error: The source directory "/tmp/ncs/nrf/samples/bluetooth/peripheral_lbs" does not exist.
Specify --help for usage, or press the help button on the CMake GUI.
FATAL ERROR: command exited with status 1: /usr/bin/cmake -DWEST_PYTHON=/usr/bin/python3 -B/tmp/ncs/nrf -S/tmp/ncs/nrf/samples/bluetooth/peripheral_lbs -GNinja -DBOARD=nrf52840dk_nrf52840
# Wow, did this really happen to me !#$@#%#^%$%

$ ls
# Nothing

$ git status
fatal: not a git repository (or any of the parent directories): .git

and the repo is gone, and ALL local work has been lost.

[mbolivar-nordic]

West build just runs pristine.cmake. It has a little bit of extra checking to try to avoid running pristine.cmake in anything that isn't a valid zephyr build directory, but I think this is a build system "bug" if it is even a bug at all, and should likely be handled there.

[mbolivar-nordic]

Fixed the title 😉

[nashif]

from pristine.cmake:

# NB: This could be dangerous to execute, it is assuming the user is
# checking that the build is out-of-source with code like this:

[tejlmand]

West build just runs pristine.cmake.

This is exactly the problem. west unconditionally tries to outsmart the user and do a pristine build as its default behavior. Even in a fresh workspace.

A user doing ninja pristine in the repo root is asking for trouble, but a user doing west build should not experience this.

So the problem is that west build tries to outsmart the user.

Changed title back, as it is west build that decided to run pristine in the first place

[tejlmand]

from pristine.cmake:

# NB: This could be dangerous to execute, it is assuming the user is
# checking that the build is out-of-source with code like this:

The problem here is not the pristine, that is documented. The problem is west build automatically doing it, even when it should not.

Suggestion, west build should never call pristine, if folder is having .git or .west.

[mbolivar-nordic]

This is exactly the problem. west unconditionally tries to outsmart the user and do a pristine build as its default behavior. Even in a fresh workspace.

If that's the problem, then I see no choice but to revert https://github.com/zephyrproject-rtos/zephyr/pull/31364.

Suggestion, west build should never call pristine, if folder is having .git or .west.

Sorry, but NAK.

I refuse to make west get into an even bigger guessing game about what it should and should not run pristine on. We can turn off build.pristine=auto by default, forcing users to specifically ask for it, but if we start this guessing game we will be playing whack-a-mole forever.

[tejlmand]

Suggestion, west build should never call pristine, if folder is having .git or .west.

Sorry, but NAK.

I refuse to make west get into an even bigger guessing game about what it should and should not run pristine on. We can turn off build.pristine=auto by default, forcing users to specifically ask for it, but if we start this guessing game we will be playing whack-a-mole forever.

IMO there is a huge difference between removing some folder with data, and then removing a complete git repository with local branches, reflog history, and so on (you removed .git so there is NO returning back). Or even worse, removing the whole west workspace with several repos.

Therefore I believe it can be justified to check for existence of .git or .west.

[mbolivar-nordic]

Maybe you misunderstood me. Nobody thinks that deleting a project repository, .west, or the workspace topdir is a good thing. That's not what I'm saying.

What I am saying is that if we refuse to pristine using code that works 99% of the time, we still have a 1% chance of deleting some file, like /path/to/file, that someone really wants, just because they incorrectly created a zephyr build directory in in a parent directory, like /path.

Then we'll chase that 1% and get something that works 99.9% of the time. And so on. No thanks.

[mbolivar-nordic]

@tejlmand if you want to add additional filters on when we run pristine, you're free to do so in pristine.cmake. It doesn't make sense to add to west build. I think #31364 addresses the issue in west build, and I've adjusted the title yet again to reflect what I think west is responsible for here.

[tejlmand]

What I am saying is that if we refuse to pristine using code that works 99% of the time, we still have a 1% chance of deleting some file, like /path/to/file, that someone really wants, just because they incorrectly created a zephyr build directory in in a parent directory, like /path.

I strongly disagree there. We know that Zephyr build system will never create a .git or .west folder, and we know that it is hazardous to remove those directoriesm as there is no coming back.

If a user creates a important-file.keep, and don't put it under version control, then that file could still risk being deleted by other means, like git clean. If the file is important, then it should be under version control.

But removing .git means that even files under version control gets removed, so I see a big difference in doing something we know is wrong in 100% of the cases, and we decide not to prevent it, and then your use-case of whether we should handle a case we cannot know about.

[tejlmand]

I think #31364 addresses the issue in west build, and I've adjusted the title yet again to reflect what I think west is responsible for here.

This is a bug PR, and the title should reflect the bug. The fact that build.pristine=auto is dangerous is not in itself a bug, just like rm -rf / is not a bug. But if mkdir / would do a rm -rf / in it's default setup, then that would be a bug.

It is a bug that west build in it's default configuration may remove a users .git folder, and the title should reflect the nature of the bug, to allow people to notice it. The fact it happens because west build defaults to pristine=auto is an implementation detailt, and that people can configure a different behavior is a workaround. pristine=auto is not in itself an error, as long as that option is an opt-in, and the fact that this option is dangerous should be highlighted in the docs.

So, as a bug title:

west build might destroy your repository, as it is defaulting doing pristine.

is correct as it do describe the outcome of the bug, and that it happens in its default setup. And this is what you fixed with: #31364