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

[update-checkout]: Add --fetch option. #25566

Closed
wants to merge 3 commits into from
Closed

[update-checkout]: Add --fetch option. #25566

wants to merge 3 commits into from

Conversation

saeta
Copy link
Contributor

@saeta saeta commented Jun 18, 2019

When checking out repositories, it's sometimes necessary to run git fetch
first to pull the latest commits, branches, and tags down from GitHub. This
commit adds a --fetch option so that the update-checkout script will
perform a git fetch first.

Note: this change preserves the default behavior, such that update-checkout can
be run without an internet connection.

When checking out repositories, it's sometimes necessary to run `git fetch`
first to pull the latest commits, branches, and tags down from GitHub. This
commit adds a `--fetch` option so that the update-checkout script will
perform a `git fetch` first.

Note: this change preserves the default behavior, such that update-checkout can
be run without an internet connection.
@jrose-apple
Copy link
Contributor

It fetches by default for me. What behavior are you observing?

@@ -94,6 +94,10 @@ def update_single_repository(args):
with shell.pushd(repo_path, dry_run=False, echo=False):
cross_repo = False
checkout_target = None

if should_fetch:
shell.run(['git', 'fetch'], echo=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think some extra arguments are necessary:

Suggested change
shell.run(['git', 'fetch'], echo=True)
shell.run(['git', 'fetch'], echo=True)
shell.run(["git", "fetch", "--recurse-submodules=yes", "--tags"],
echo=True)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, this exact git fetch command is executed a few lines later in the file:

            if checkout_target:
                shell.run(['git', 'status', '--porcelain', '-uno'],
                          echo=False)
                shell.run(['git', 'checkout', checkout_target], echo=True)

            # It's important that we checkout, fetch, and rebase, in order.
            # .git/FETCH_HEAD updates the not-for-merge attributes based on
            # which branch was checked out during the fetch.
            shell.run(["git", "fetch", "--recurse-submodules=yes", "--tags"],
                      echo=True)

But maybe it's being run too late?

@dan-zheng
Copy link
Collaborator

dan-zheng commented Jun 18, 2019

It fetches by default for me. What behavior are you observing?

The behavior I've observed is: after updating Git hashes in utils/update_checkout/update-checkout-config.json, swift/utils/update-checkout --clone --scheme tensorflow doesn't fetch and fails.

diff --git a/utils/update_checkout/update-checkout-config.json b/utils/update_checkout/update-checkout-config.json
index d147ca1722..82b071f287 100644
--- a/utils/update_checkout/update-checkout-config.json
+++ b/utils/update_checkout/update-checkout-config.json
@@ -355,29 +355,29 @@
         "tensorflow": {
             "aliases": ["tensorflow"],
             "repos": {
-                "llvm": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
+                "llvm": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
                 "clang": "ea2e6942a25eb7201d32c7c21b1dd7e5658ba1e8",
                 "swift": "tensorflow",
                 "lldb": "4514d21fbe49a16baf26ba918c5a4906d54e8091",
-                "cmark": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "llbuild": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "swiftpm": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
+                "cmark": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "llbuild": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "swiftpm": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
                 "swift-syntax": "ce19fd53c3137baa1c3370d9577c18f125934a04",
-                "swift-stress-tester": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "compiler-rt": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "swift-corelibs-xctest": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "swift-corelibs-foundation": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "swift-corelibs-libdispatch": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "swift-integration-tests": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "swift-xcode-playground-support": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
+                "swift-stress-tester": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "compiler-rt": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "swift-corelibs-xctest": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "swift-corelibs-foundation": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "swift-corelibs-libdispatch": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "swift-integration-tests": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "swift-xcode-playground-support": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
                 "ninja": "release",
                 "icu": "release-61-1",
-                "clang-tools-extra": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "libcxx": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
+                "clang-tools-extra": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "libcxx": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
                 "tensorflow": "ebc41609e27dcf0998d8970e77a2e1f53e13ac86",
                 "tensorflow-swift-apis": "5d3ef57b501781f1bab3b4ca85e8b8fc91671c14",
-                "indexstore-db": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a",
-                "sourcekit-lsp": "swift-DEVELOPMENT-SNAPSHOT-2019-06-06-a"
+                "indexstore-db": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a",
+                "sourcekit-lsp": "swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a"
             }
         }
     }

Previously, all repositories were checked out at swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a and have not been fetched since then.

$ /swift/utils/update-checkout --clone --scheme tensorflow --skip-repository swift
Including icu on Linux
Skipping clone of 'sourcekit-lsp', directory already exists
Skipping clone of 'indexstore-db', directory already exists
Skipping clone of 'llbuild', directory already exists
Skipping clone of 'swift-xcode-playground-support', directory already exists
Skipping clone of 'swift-syntax', directory already exists
Skipping clone of 'swift-corelibs-xctest', directory already exists
Skipping clone of 'swiftpm', directory already exists
Skipping clone of 'compiler-rt', directory already exists
Skipping clone of 'libcxx', directory already exists
Skipping clone of 'tensorflow-swift-apis', directory already exists
Skipping clone of 'lldb', directory already exists
Skipping clone of 'tensorflow', directory already exists
Skipping clone of 'icu', directory already exists
Skipping clone of 'llvm', directory already exists
Skipping clone of 'clang', directory already exists
Skipping clone of 'swift-corelibs-foundation', directory already exists
Skipping clone of 'ninja', directory already exists
Skipping clone of 'swift-integration-tests', directory already exists
Skipping clone of 'swift', requested by user
Skipping clone of 'clang-tools-extra', directory already exists
Skipping clone of 'swift-stress-tester', directory already exists
Skipping clone of 'cmark', directory already exists
Skipping clone of 'swift-corelibs-libdispatch', directory already exists
Not cloning any repositories.
Skipping update of 'swift', requested by user
Running ``update_single_repository`` with up to 192 processes.
Updating '/home/danielzheng/swift-merge/sourcekit-lsp'
Updating '/home/danielzheng/swift-merge/indexstore-db'
Updating '/home/danielzheng/swift-merge/llbuild'
Updating '/home/danielzheng/swift-merge/swift-xcode-playground-support'
Updating '/home/danielzheng/swift-merge/swift-syntax'
Updating '/home/danielzheng/swift-merge/swift-corelibs-xctest'
Updating '/home/danielzheng/swift-merge/swiftpm'
Updating '/home/danielzheng/swift-merge/compiler-rt'
Updating '/home/danielzheng/swift-merge/libcxx'
Updating '/home/danielzheng/swift-merge/tensorflow-swift-apis'
Updating '/home/danielzheng/swift-merge/lldb'
Updating '/home/danielzheng/swift-merge/tensorflow'
Updating '/home/danielzheng/swift-merge/icu'
Updating '/home/danielzheng/swift-merge/llvm'
Updating '/home/danielzheng/swift-merge/clang'
Updating '/home/danielzheng/swift-merge/swift-corelibs-foundation'
Updating '/home/danielzheng/swift-merge/ninja'
Updating '/home/danielzheng/swift-merge/swift-integration-tests'
Updating '/home/danielzheng/swift-merge/clang-tools-extra'
/home/danielzheng/swift-merge/sourcekit-lsp
+ git checkout swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swift-xcode-playground-support
Updating '/home/danielzheng/swift-merge/swift-stress-tester'
+ git checkout swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/indexstore-db
Error on repo "/home/danielzheng/swift-merge/sourcekit-lsp": Traceback (most recent call last):
  File "/home/danielzheng/swift-merge/swift/utils/update_checkout/update_checkout/update_checkout.py", line 128, in update_single_repository
    shell.run(['git', 'checkout', checkout_target], echo=True)
  File "/home/danielzheng/swift-merge/swift/utils/swift_build_support/swift_build_support/shell.py", line 231, in run
    raise eout
Exception: ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
+ git checkout swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a

error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

Updating '/home/danielzheng/swift-merge/cmark'
Error on repo "/home/danielzheng/swift-merge/swift-xcode-playground-support": Traceback (most recent call last):
  File "/home/danielzheng/swift-merge/swift/utils/update_checkout/update_checkout/update_checkout.py", line 128, in update_single_repository
    shell.run(['git', 'checkout', checkout_target], echo=True)
  File "/home/danielzheng/swift-merge/swift/utils/swift_build_support/swift_build_support/shell.py", line 231, in run
    raise eout
Exception: ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']

... # more failures

======UPDATE FAILURES======
/home/danielzheng/swift-merge/sourcekit-lsp failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/indexstore-db failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/llbuild failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swift-xcode-playground-support failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swift-corelibs-xctest failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swiftpm failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/compiler-rt failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/libcxx failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/llvm failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swift-corelibs-foundation failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swift-integration-tests failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/clang-tools-extra failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swift-stress-tester failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/cmark failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

/home/danielzheng/swift-merge/swift-corelibs-libdispatch failed (ret=1): ['git', 'checkout', u'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a']
error: pathspec 'swift-DEVELOPMENT-SNAPSHOT-2019-06-16-a' did not match any file(s) known to git.

update-checkout failed, fix errors and try again

With this patch, passing --fetch makes the command succeed:

$ ./swift/utils/update-checkout --clone --scheme tensorflow --fetch
...
update-checkout succeeded

I wonder if others have observed this behavior? (the Swift for TensorFlow team has)

@saeta saeta requested a review from jrose-apple June 18, 2019 16:14
@saeta
Copy link
Contributor Author

saeta commented Jun 18, 2019

It fetches by default for me. What behavior are you observing?

Concretely, it looks like there's a git fetch happening here but it's happening after the git checkout which is the problem as the tags and latest commits have not been pulled down.

Thank you very much for taking a look @jrose-apple !

All the best,
-Brennan

@jrose-apple
Copy link
Contributor

Ah, I guess I don't use update-checkout to change schemes very often. But the very next lines say that this is a bad idea.

# It's important that we checkout, fetch, and rebase, in order.
# .git/FETCH_HEAD updates the not-for-merge attributes based on
# which branch was checked out during the fetch.

…unfortunately, the person who wrote them is no longer on the project.

@saeta
Copy link
Contributor Author

saeta commented Jul 4, 2019

PTAL @jrose-apple / @dan-zheng . I've switched over to using git remote update which should do what we want without stomping on .git/FETCH_HEAD based on my read of the git documentation. Thanks!

@jrose-apple
Copy link
Contributor

This seems reasonable to me. @gottesmm?

@gottesmm
Copy link
Member

gottesmm commented Jul 5, 2019

Can you add a unit test for this? I recently provided the infrastructure to create mock repos/etc. Every new update-checkout change that we put in should at least have a test that makes sure it runs without asserting. Something like this:

https://github.com/apple/swift/blob/master/utils/update_checkout/tests/test_dump.py

I haven't messed around with checking the actual output, but I imagine it would be pretty easy to add such checks.

@CodaFi
Copy link
Member

CodaFi commented Apr 15, 2020

I'm going to close this out due to age and inactivity. I think it's still a change worth pursuing, so I've converted this into an SR. Please take a look at it when you find the time.

@CodaFi CodaFi closed this Apr 15, 2020
@AnthonyLatsis AnthonyLatsis added the update-checkout Area → utils: the `update-checkout` script label Sep 28, 2022
@AnthonyLatsis AnthonyLatsis linked an issue Oct 19, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
update-checkout Area → utils: the `update-checkout` script
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SR-12585] Teach update-checkout to fetch Before checkout
6 participants