west update checks out incorrectly

[Rallare]

OS: Windows
Version: 0.6.1

If you go back-and-forth between two tags (lets say "v1.0.0" and "master"), "west update" seems to cache the former west.yaml files, so that you get a mismatch between the nrf repo, and the surrounding git repo deps.

Here's an example:
haal@haal MINGW64 /c/SDK/ncs/nrf (master)
$ west update
=== updating zephyr (zephyr):
--- zephyr: checked out 7e64ac737daead12d2dbdda5daec04188243a81c as detached HEAD

that commit is the "v1.14.99-ncs2-branch", corresponding to nrf tag "v1.0.0".
However; west.yaml shows this revision for fw-nrfconnect-zephyr:
bd8fd7f8982a2a2b03d65e415b8de17b94fa5a9a

[E3V3A]

Yeah, this is pretty serious, as it prevents correct behavior for all the nRF compilation libraries, who seem to rely solely on west for installation.

[mbolivar]

Fixed in 0.6.2; please use it instead of 0.6.1. Versions 0.6.0 and earlier are not affected.

[marc-hb]

I'm in the middle of a large rebase so I can't really west update or update west itself, I mean not yet.

If you go back-and-forth between two tags (lets say "v1.0.0" and "master"),

So this bug doesn't affect anyone who only ever goes "forth" on all the master branches and never "back or sideways". Can you confirm? Thanks!

[mbolivar]

So this bug doesn't affect anyone who only ever goes "forth" on all the master branches and never "back or sideways". Can you confirm? Thanks!

Yes, that is my expectation.

The details on this since I know you will read them at least 😄 are:

• West version 0.6.1 introduced a new feature I'm calling the "fetch optimization"; its purpose is to skip fetching from a project remote when its revision is a tag or a SHA that is already available locally
• In 0.6.1 (but NOT 0.6.2), when this optimization takes effect -- and only then -- I forgot to update the manifest-rev git ref which west uses to track the latest manifest revision in each project. When the project is fetched over the network, manifest-rev is updated correctly.
• After the fetch step, west checks out the manifest-rev ref as a detached HEAD. This means that when the user has gone "backwards" or "sideways" in the manifest repository and then runs west update, the optimization takes effect, but the manifest-rev revision is stale. In fact, it points at the ref for the most recent time the project revision had to be fetched from the network.

All of this can be put together to see that if you only go "forwards" in time in your manifest repository, the manifest-rev branch is always correct:

• if you pull and west update needs to fetch, it keeps manifest-rev in sync, so checking out that revision works
• if you pull and west update does not need to fetch, then because you've only gone forwards in time, that means manifest-rev is already pointing at the correct ref (if the revision is available locally, it must be the latest one), so the update still works correctly

The fix, of course, is to update manifest-rev no matter what happens, so the subsequent checkout works.

[marc-hb]

then because you've only gone forwards in time, that means manifest-rev is already pointing at the correct ref (if the revision is available locally, it must be the latest one)

Must it be? Here's what I have right now in my local ci-tools module. That may not be the latest github state (it actually is as of 2019-09-04) but it was at some point:

    d572f3df7513 carles.cufi@nordicsemi.no // (refs/west/master, upstream/master) what_changed: Add entry for Bluetooth Mesh
    640744b11912 Ulf.Magnusson@nordicsemi.no // check_compliance: Run the dtlib and edtlib test suites in CI
    d56f2dd3510e anas.nashif@intel.com // (HEAD, manifest-rev) compliance: clarify CODEOWNER message
    e4e846c66996 anas.nashif@intel.com // Revert "codeowners: temporarily disable codeowner failures"
    700265513296 anas.nashif@intel.com // codeowner: do not allow commas

If manifest-rev then gets fast forwarded to upstream/master, could this bug reproduce?

[mbolivar]

I should add that one workaround to avoid this bug on v0.6.1 is to use west update --fetch=always.

[mbolivar]

If manifest-rev then gets fast forwarded to upstream/master, could this bug reproduce?

Ah, yes, I think your analysis is right and my "expectation" was wrong. I was making a (faulty) assumption the manifest revision would always be up to date with the remote master branch (or more generally refs/heads/*) whenever the user fetches. This is not correct in your example, which is a perfectly valid use case.

I guess you mean something along the lines of:

#. fetch upstream ci-tools refs/heads/* (with or without west). revision d572f3df7513 is now available locally.
#. update west.yml revision for this project from d56f2dd3510e to d572f3df7513
#. run west update

In that case, yes, fetch optimization kicks in, but in 0.6.1 manifest-rev incorrectly stays at d56f2dd3510e, reproducing the bug. Ugh, sorry.

As mentioned in my other comment, the easiest way to skip the bug entirely on 0.6.1 is by using west update --fetch=always.

I've verified your example works correctly on 0.6.1 with that command. I.e., d572f3df751359126ed324601ebc252402fe3a7e is checked out afterwards if I write that into the project revision in west.yml with manifest-rev pointing d56f2dd3510e, then run west update --fetch=always.

I've also verified it's working correctly on 0.6.2 with or without --fetch=always.

TL;DR: the bug manifests itself in more cases than I originally thought, but the fix as described is good, and you can avoid it entirely on 0.6.1 using west update --fetch=always.

[eabase]

Just updated, and the response is that the old files need to be overwritten since west is using detached HEAD branch. What is the best solution?

C:\NordicS\ncs\nrf>west -V
West version: v0.6.2

C:\NordicS\ncs\nrf>west update
=== updating fw-nrfconnect-zephyr (zephyr):
error: Your local changes to the following files would be overwritten by checkout:
        arch/common/gen_isr_tables.py
        arch/x86/gen_mmu_x86.py
        arch/xtensa/core/xtensa_intgen.py
        boards/x86/common/scripts/build_grub.sh
        ext/hal/nxp/mcux/scripts/import_mcux_sdk.py
        scripts/ci/get_modified_boards.py
        scripts/ci/get_modified_tests.py
        scripts/ci/run_ci.sh
        scripts/coccicheck
        scripts/dts/devicetree.py
        scripts/dts/extract_dts_includes.py
        scripts/dts/gen_defines.py
        scripts/dts/testdtlib.py
        scripts/dts/testedtlib.py
        scripts/filter-known-issues.py
        scripts/footprint/size_report
        scripts/gen_cfb_font_header.py
        scripts/gen_kobject_list.py
        scripts/gen_priv_stacks.py
        scripts/kconfig/guiconfig.py
        scripts/kconfig/kconfig.py
        scripts/kconfig/menuconfig.py
        scripts/sanitycheck
        scripts/zephyr_module.py
Please commit your changes or stash them before you switch branches.
Aborting
=== updating nffs (modules\fs\nffs):
--- nffs: checked out bc62a2fa9d98ddb5d633c932ea199bc68e10f194 as detached HEAD
=== updating segger (modules\debug\segger):
--- segger: checked out 6fcf61606d6012d2c44129edc033f59331e268bc as detached HEAD
=== updating fw-nrfconnect-mcuboot (mcuboot):
error: Your local changes to the following files would be overwritten by checkout:
        boot/mynewt/src/main.c
        scripts/imgtool/main.py
Please commit your changes or stash them before you switch branches.
Aborting
=== updating fw-nrfconnect-mcumgr (modules\lib\mcumgr):
--- fw-nrfconnect-mcumgr: checked out c8f675a5fa7f62106b9d913844bd5ed6e16ae69e as detached HEAD
=== updating fw-nrfconnect-tinycbor (modules\lib\tinycbor):
--- fw-nrfconnect-tinycbor: checked out 543ecb7c8662580ef803d59ceda7bd3b8a84a11b as detached HEAD
=== updating nrfxlib (nrfxlib):
--- nrfxlib: checked out efcbc258d7c869457e5af59f100fe9a7eeb19b62 as detached HEAD
=== updating cmock (test\cmock):
--- cmock: checked out c243b9a7a7b3c471023193992b46cf1bd1910450 as detached HEAD
=== updating unity (test\cmock\vendor\unity):
--- unity: checked out 031f3bbe45f8adf504ca3d13e6f093869920b091 as detached HEAD
=== updating mbedtls (mbedtls):
error: Your local changes to the following files would be overwritten by checkout:
        scripts/abi_check.py
        scripts/bump_version.sh
        scripts/config.pl
        scripts/generate_errors.pl
        scripts/generate_query_config.pl
        scripts/generate_visualc_files.pl
        tests/compat-in-docker.sh
        tests/compat.sh
        tests/git-scripts/pre-push.sh
        tests/make-in-docker.sh
        tests/scripts/all-in-docker.sh
        tests/scripts/all.sh
        tests/scripts/basic-build-test.sh
        tests/scripts/basic-in-docker.sh
        tests/scripts/check-files.py
        tests/scripts/check-generated-files.sh
        tests/scripts/check-names.sh
        tests/scripts/check-python-files.sh
        tests/scripts/curves.pl
        tests/scripts/docker_env.sh
        tests/scripts/generate_test_code.py
        tests/scripts/list-enum-consts.pl
        tests/scripts/list-identifiers.sh
        tests/scripts/list-macros.sh
        tests/scripts/list-symbols.sh
        tests/scripts/mbedtls_test.py
        tests/scripts/run-test-suites.pl
        tests/scripts/test_generate_test_code.py
        tests/ssl-opt-in-docker.sh
        tests/ssl-opt.sh
Please commit your changes or stash them before you switch branches.
Aborting
ERROR: update failed for projects: fw-nrfconnect-zephyr, fw-nrfconnect-mcuboot, mbedtls

C:\NordicS\ncs\nrf>git branch
* (HEAD detached at v1.0.0)
  master

I also tried with west update --fetch=always, same problem.

[mbolivar]

error: Your local changes to the following files would be overwritten by checkout:

This is just a generic git error you get when trying to run git checkout with a dirty working tree; nothing west specific to see here as far as I can tell.

What is the best solution?

See the command output lower down:

Please commit your changes or stash them before you switch branches.

[eabase]

Since there is nothing of interest to stash, the best solution is to overwrite. Just do:

cd /pathto/ncs/nrf
git checkout -f master
git pull
west update