Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1147033003:
Rename roll-dep to roll-dep-svn and add roll-dep for git. (Closed)
DescriptionRename roll-dep to roll-dep-svn and add roll-dep for git.
It is inspired by src/tools/roll_swarming_client.py but generalized.
R=dpranke@chromium.org, stip@chromium.org
BUG=
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=295386
Patch Set 1 #
Total comments: 13
Patch Set 2 : improved #
Total comments: 1
Messages
Total messages: 20 (5 generated)
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
https://codereview.chromium.org/1147033003/diff/1/roll_dep_svn.py File roll_dep_svn.py (right): https://codereview.chromium.org/1147033003/diff/1/roll_dep_svn.py#newcode6 roll_dep_svn.py:6: """This scripts takes the path to a dep and a git or svn revision, and updates Do we really want this script to support taking git revisions? Does this script work for rolling a pure-git repo, or does it need git-svn-id's to be present?
lgtm with nits, please address dirk's comment https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py File roll_dep.py (right): https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode6 roll_dep.py:6: """Rolls DEPS controled dependency. controlled https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode19 roll_dep.py:19: """Returns True is a git checkout is pristine.""" if https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode32 roll_dep.py:32: print >> sys.stderr, 'Ensure to be in directory containing DEPS file.' ensure the script is run in the directory containing the DEPS file https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode97 roll_dep.py:97: subprocess.check_call(['git', 'add', 'DEPS'], cwd=root) wow https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode105 roll_dep.py:105: print(' git cl upl --send-mail') probably want git cl upload just in case https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep_svn.py File roll_dep_svn.py (right): https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep_svn.py#newc... roll_dep_svn.py:6: """This scripts takes the path to a dep and a git or svn revision, and updates On 2015/05/20 20:03:51, Dirk Pranke wrote: > Do we really want this script to support taking git revisions? Does this script > work for rolling a pure-git repo, or does it need git-svn-id's to be present? Yeah, please update the doc here
https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py File roll_dep.py (right): https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode6 roll_dep.py:6: """Rolls DEPS controled dependency. On 2015/05/20 23:08:16, stip wrote: > controlled Done. https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode19 roll_dep.py:19: """Returns True is a git checkout is pristine.""" On 2015/05/20 23:08:16, stip wrote: > if Done. https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode32 roll_dep.py:32: print >> sys.stderr, 'Ensure to be in directory containing DEPS file.' On 2015/05/20 23:08:16, stip wrote: > ensure the script is run in the directory containing the DEPS file Done. https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep.py#newcode105 roll_dep.py:105: print(' git cl upl --send-mail') On 2015/05/20 23:08:16, stip wrote: > probably want git cl upload just in case Done. https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep_svn.py File roll_dep_svn.py (right): https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep_svn.py#newc... roll_dep_svn.py:6: """This scripts takes the path to a dep and a git or svn revision, and updates On 2015/05/20 20:03:51, Dirk Pranke wrote: > Do we really want this script to support taking git revisions? Does this script > work for rolling a pure-git repo, or does it need git-svn-id's to be present? There is still sadly git-svn repositories still mapped, that's why I moved it aside instead of deleting it. https://chromiumcodereview.appspot.com/1147033003/diff/1/roll_dep_svn.py#newc... roll_dep_svn.py:6: """This scripts takes the path to a dep and a git or svn revision, and updates On 2015/05/20 23:08:16, stip wrote: > On 2015/05/20 20:03:51, Dirk Pranke wrote: > > Do we really want this script to support taking git revisions? Does this > script > > work for rolling a pure-git repo, or does it need git-svn-id's to be present? > > Yeah, please update the doc here Done
On 2015/05/21 00:10:52, M-A Ruel wrote: > There is still sadly git-svn repositories still mapped, that's why I moved it > aside instead of deleting it. Understood. I'm not sure the names "roll_dep" and "roll_dep_svn" make it clear that you use the former for a pure git repo, the latter for a git+svn repo, and that neither works for an svn-only repo (or does the latter work for svn-only? do we even support svn-only?) -- Dirk
On 2015/05/21 00:46:10, Dirk Pranke wrote: > On 2015/05/21 00:10:52, M-A Ruel wrote: > > There is still sadly git-svn repositories still mapped, that's why I moved it > > aside instead of deleting it. > > Understood. I'm not sure the names "roll_dep" and "roll_dep_svn" make it clear > that you use the former for a pure git repo, the latter for a git+svn repo, and > that neither works for an svn-only repo (or does the latter work for svn-only? > do we even support svn-only?) To be clear, I never used this script but maybe someone did, so I didn't want to delete the script right away.
On 2015/05/21 00:56:35, M-A Ruel wrote: > On 2015/05/21 00:46:10, Dirk Pranke wrote: > > On 2015/05/21 00:10:52, M-A Ruel wrote: > > > There is still sadly git-svn repositories still mapped, that's why I moved > it > > > aside instead of deleting it. > > > > Understood. I'm not sure the names "roll_dep" and "roll_dep_svn" make it clear > > that you use the former for a pure git repo, the latter for a git+svn repo, > and > > that neither works for an svn-only repo (or does the latter work for svn-only? > > do we even support svn-only?) > > To be clear, I never used this script but maybe someone did, so I didn't want to > delete the script right away. Dirk, any remaining concern?
good enough, I suppose :) lgtm.
The CQ bit was checked by maruel@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stip@chromium.org Link to the patchset: https://codereview.chromium.org/1147033003/#ps20001 (title: "improved")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1147033003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for depot_tools/roll-dep-svn.bat: While running patch -p1 --forward --force --no-backup-if-mismatch; A depot_tools/roll-dep-svn.bat Copied depot_tools/roll-dep.bat -> depot_tools/roll-dep-svn.bat patching file depot_tools/roll-dep-svn.bat Hunk #1 FAILED at 7 (different line endings). 1 out of 1 hunk FAILED -- saving rejects to file depot_tools/roll-dep-svn.bat.rej Patch: NR depot_tools/roll-dep.bat->depot_tools/roll-dep-svn.bat Index: roll-dep-svn.bat diff --git depot_tools/roll-dep.bat depot_tools/roll-dep-svn.bat similarity index 83% copy from depot_tools/roll-dep.bat copy to depot_tools/roll-dep-svn.bat index d8b69c27c2efb6c0e806f2fa8aad225a66c01291..01e8287835fd212e6c80e520a5f8a0c6a0a1f92f 100755 --- a/depot_tools/roll-dep.bat +++ b/depot_tools/roll-dep-svn.bat @@ -7,4 +7,4 @@ PATH=%~dp0;%PATH% :: Defer control. -%~dp0python "%~dp0\roll_dep.py" %* +%~dp0python "%~dp0\roll_dep_svn.py" %*
On 2015/05/22 18:38:21, I haz the power - commit-bot wrote: > Failed to apply patch for depot_tools/roll-dep-svn.bat: > While running patch -p1 --forward --force --no-backup-if-mismatch; > A depot_tools/roll-dep-svn.bat /me raises fist at svn and commits manually.
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 295386.
Message was sent while issue was closed.
smut@google.com changed reviewers: + smut@google.com
Message was sent while issue was closed.
https://codereview.chromium.org/1147033003/diff/20001/roll_dep.py File roll_dep.py (right): https://codereview.chromium.org/1147033003/diff/20001/roll_dep.py#newcode31 roll_dep.py:31: except OSError: Need to except IOError as well. I'm getting IOError: [Errno 2] No such file or directory.
Message was sent while issue was closed.
The v8 auto-roller is now broken. It uses roll_dep. Is this connected? https://build.chromium.org/p/client.v8.fyi/builders/Auto-roll%20-%20deps/buil...
Message was sent while issue was closed.
Can you switch it to use roll-dep-svn instead? I'm currently away from computer. Wrote by human Le 2015-05-23 9:10 AM, <machenbach@chromium.org> a écrit : > The v8 auto-roller is now broken. It uses roll_dep. Is this connected? > > https://build.chromium.org/p/client.v8.fyi/builders/Auto-roll%20-%20deps/buil... > > https://codereview.chromium.org/1147033003/ > To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
