Opened 8 months ago

Closed 7 months ago

#28784 closed defect (fixed)

Assembling WebRTC sources fails with error "You have unstaged changes"

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm, snowflake, TorBrowserTeam201812R
Cc: dcf, boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

After testing the patches in #28725 and merging them to master I decided to get rid of all by built snowflake related artifacts and deleted out/snowflake, out/go-webrtc and out/webrtc and check if everything is still working.

That does not seem to be the case as I am already getting errors during WebRTC sources assembling now:

Starting build: Fri Dec  7 14:05:23 2018
Bootstrapping cipd client for linux-amd64 from https://chrome-infra-packages.appspot.com/client?platform=linux-amd64&version=git_revision:d2677a4477e59cb7de00f1fb8a00e96b1aaeb927...

src/testing (ERROR)
----------------------------------------
[0:00:02] Started.
[0:00:07] From https://chromium.googlesource.com/chromium/src/testing
[0:00:07]    a5684e6..36a1586  master     -> origin/master
----------------------------------------
Error: 5> 
5> ____ src/testing at 60c665fffe7dc505fdd5d30f9dbcbc50dde1e017
5> 	You have unstaged changes.
5> 	Please commit, stash, or reset.

I wonder if that's a thing for boklm's idea about cleaning up old files, expressed in comment:9:ticket:28725.

Child Tickets

Attachments (1)

0001-Bug-28784-Pass-force-delete_unversioned_trees-reset-.patch (2.1 KB) - added by dcf 7 months ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 8 months ago by gk

Indeed. Here is what I am seeing:

HEAD detached at 60c665f
Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        deleted:    gmock/BUILD.gn
        deleted:    gmock/OWNERS
        deleted:    gmock/include/gmock/gmock-actions.h
        deleted:    gmock/include/gmock/gmock-generated-function-mockers.h
        deleted:    gmock/include/gmock/gmock-matchers.h
        deleted:    gmock/include/gmock/gmock.h
        deleted:    gtest/BUILD.gn
        deleted:    gtest/OWNERS
        deleted:    gtest/empty.cc
        deleted:    gtest/include/gtest/gtest-death-test.h
        deleted:    gtest/include/gtest/gtest-message.h
        deleted:    gtest/include/gtest/gtest-param-test.h
        deleted:    gtest/include/gtest/gtest-spi.h
        deleted:    gtest/include/gtest/gtest.h
        deleted:    gtest/include/gtest/gtest_prod.h

comment:2 Changed 8 months ago by gk

We did

+      # Delete the unversioned gmock and gtest directories, which cause "gclient sync"
+      # to fail when upgrading between webrtc branch-heads/58 and branch-heads/64.
+      rm -rf src/testing/gmock src/testing/gtest

but it seems those directories *are* versioned instead?

comment:3 Changed 8 months ago by dcf

I see this too. I'll see what I can do.

Upstream go-webrtc does a `git reset --hard HEAD`, but I think that's insufficient because there are multiple nested repositories.

comment:4 Changed 7 months ago by dcf

Status: newneeds_review

I found some helpful options of gclient sync:

$ gclient help sync
  -f, --force           force update even for unchanged modules
  -D, --delete_unversioned_trees
                        Deletes from the working copy any dependencies that
                        have been removed since the last sync, as long as
                        there are no local modifications. When used with
                        --force, such dependencies are removed even if they
                        have local modifications. When used with --reset, all
                        untracked directories are removed from the working
                        copy, excluding those which are explicitly ignored in
                        the repository.
  -R, --reset           resets any local changes before updating (git only)

Adding these three options seems to take care of the gmock and gtest directories, no matter their current status.

I tested this with a gclient/webrtc that had been manually set up with branch-heads/58 (gclient sync --nohooks --no-history --with_branch_heads -r c279861207c5b15fc51069e96595782350e0ac12) in order to simulate a webrtc upgrade, and I also tested after manually deleting src/testing/gmock and src/testing/gtest.

comment:5 Changed 7 months ago by gk

Keywords: TorBrowserTeam201812R added
Resolution: fixed
Status: needs_reviewclosed

Looks good to me (ans solves the issue I had). Applied to master (1b7e0de3690364bfdd29889676f5ff8ec6b1cd3b) with a slightly modified commit message (fixing a typo and adjusting the help output a bit). Thanks!

Note: See TracTickets for help on using tickets.