branch-util broken at ToT |
|||||||||||||||||
Issue description
We were trying to branch from recent ToT for factory branch.
The local dry run looks good, with,
cros tryjob branch-util-tryjob --local --branch-name=factory-poppy-10509.B --version=10509.0.0
On the final run,
cros tryjob branch-util --local --production --branch-name=factory-poppy-10509.B --version=10509.0.0
Both of us saw something similar to this,
cbuildbot: Unhandled exception:
Traceback (most recent call last):
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/bin/cbuildbot", line 169, in <module>
DoMain()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/bin/cbuildbot", line 165, in DoMain
commandline.ScriptWrapperMain(FindTarget)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/commandline.py", line 911, in ScriptWrapperMain
ret = target(argv[1:])
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/scripts/cbuildbot.py", line 1019, in main
logging.info('One stage exited early: %s', ex)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/scripts/cbuildbot.py", line 981, in main
stack.Add(_SetupConnections, options, build_config)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cros_build_lib.py", line 1331, in Add
obj = functor(*args, **kwargs)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/scripts/cbuildbot.py", line 803, in _SetupConnections
db = cidb.CIDBConnectionFactory.GetCIDBConnectionForBuilder()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 2197, in GetCIDBConnectionForBuilder
return self.GetInstance()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/factory.py", line 104, in GetInstance
return self._types[self.setup_type]()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/factory.py", line 120, in wrapper
cached_value.append(function())
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 2090, in <lambda>
lambda: CIDBConnection(constants.CIDB_PROD_BOT_CREDS)),
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 696, in __init__
for_service=for_service, query_retry_args=query_retry_args)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 270, in __init__
listeners=[self._listener_class()])
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/__init__.py", line 387, in create_engine
return strategy.create(*args, **kwargs)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/strategies.py", line 80, in create
dbapi = dialect_cls.dbapi(**dbapi_args)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/dialects/mysql/mysqldb.py", line 110, in dbapi
return __import__('MySQLdb')
ImportError: No module named MySQLdb
We also tried few other versions (e.g. 10504.0.0, 10510.0.0) with the same problem.
Just wondering if we had missed something?
Thanks.
,
Mar 23 2018
If this was a standalone tool, MySql wouldn't be needed at all.
,
Mar 23 2018
RE: #1
Not much better, unfortunately.
Traceback (most recent call last):
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/bin/cbuildbot", line 169, in <module>
DoMain()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/bin/cbuildbot", line 165, in DoMain
commandline.ScriptWrapperMain(FindTarget)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/commandline.py", line 911, in ScriptWrapperMain
ret = target(argv[1:])
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/scripts/cbuildbot.py", line 1019, in main
logging.info('One stage exited early: %s', ex)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/scripts/cbuildbot.py", line 981, in main
stack.Add(_SetupConnections, options, build_config)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cros_build_lib.py", line 1331, in Add
obj = functor(*args, **kwargs)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/scripts/cbuildbot.py", line 803, in _SetupConnections
db = cidb.CIDBConnectionFactory.GetCIDBConnectionForBuilder()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 2197, in GetCIDBConnectionForBuilder
return self.GetInstance()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/factory.py", line 104, in GetInstance
return self._types[self.setup_type]()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/factory.py", line 120, in wrapper
cached_value.append(function())
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 2090, in <lambda>
lambda: CIDBConnection(constants.CIDB_PROD_BOT_CREDS)),
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 696, in __init__
for_service=for_service, query_retry_args=query_retry_args)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 273, in __init__
temp_engine).fetchall()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 577, in _ExecuteWithEngine
return self._RunFunctorWithRetries(f)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 596, in _RunFunctorWithRetries
functor=functor)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/retry_stats.py", line 181, in RetryWithStats
*args, **kwargs)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/retry_util.py", line 244, in GenericRetry
return _run()
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/retry_util.py", line 177, in _Wrapper
ret = func(*args, **kwargs)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/retry_util.py", line 243, in _run
return functor(*args, **kwargs)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/retry_stats.py", line 168, in wrapper
result = functor(*args, **kwargs)
File "/usr/local/google/home/yueherngl/tryjob/repository/chromite/lib/cidb.py", line 573, in <lambda>
f = lambda: engine.execute(query, *args, **kwargs)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2063, in execute
connection = self.contextual_connect(close_with_result=True)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2112, in contextual_connect
self._wrap_pool_connect(self.pool.connect, None),
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2151, in _wrap_pool_connect
e, dialect, self)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 1465, in _handle_dbapi_exception_noconnection
exc_info
File "/usr/lib/python2.7/dist-packages/sqlalchemy/util/compat.py", line 203, in raise_from_cause
reraise(type(exception), exception, tb=exc_tb, cause=cause)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/base.py", line 2147, in _wrap_pool_connect
return fn()
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 387, in connect
return _ConnectionFairy._checkout(self)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 766, in _checkout
fairy = _ConnectionRecord.checkout(pool)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 516, in checkout
rec = pool._do_get()
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 1138, in _do_get
self._dec_overflow()
File "/usr/lib/python2.7/dist-packages/sqlalchemy/util/langhelpers.py", line 66, in __exit__
compat.reraise(exc_type, exc_value, exc_tb)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 1135, in _do_get
return self._create_connection()
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 333, in _create_connection
return _ConnectionRecord(self)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 461, in __init__
self.__connect(first_connect_check=True)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/pool.py", line 651, in __connect
connection = pool._invoke_creator(self)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/strategies.py", line 105, in connect
return dialect.connect(*cargs, **cparams)
File "/usr/lib/python2.7/dist-packages/sqlalchemy/engine/default.py", line 393, in connect
return self.dbapi.connect(*cargs, **cparams)
File "/usr/lib/python2.7/dist-packages/MySQLdb/__init__.py", line 86, in Connect
return Connection(*args, **kwargs)
File "/usr/lib/python2.7/dist-packages/MySQLdb/connections.py", line 204, in __init__
super(Connection, self).__init__(*args, **kwargs2)
sqlalchemy.exc.OperationalError: (_mysql_exceptions.OperationalError) (2002, 'Can\'t connect to local MySQL server through socket \'/var/run/mysqld/mysqld.sock\' (2 "No such file or directory")')
,
Mar 27 2018
I can work around the problem by using chromite from older branch (e.g. firmware-glados-7820.B), and do ./cbuildbot branch-util --local --branch-name=factory-poppy-10504.B --version=10504.0.0 to create the branch.
,
Apr 1 2018
I followed comment 4 to create soraka factory branch but I found branches are not created for some repos. For example, src/aosp/system/connectivity/shill and src/aosp/system/update_engine. Once some CLs need to be cherry-picked, not sure whether we need to manually create the branch.
,
Apr 1 2018
RE: #6 I thought you were create the branch from R65 branch point? If so and the number of the repositories are small, maybe we can use manually create the branch directly.
,
Apr 13 2018
Over to Charles to triage for Build area. See comment #4: need to revive a broken script that was being used to create branches. A few reports on chromeos-infra-discuss, too. CBuildBot seems like the wrong place for this (note the MySQL connections for no reason). Recently, folks have been running a "cros tryjob --local --production" to invoke the cbuildbot logic on their own machine but reaching out to Prod.
,
Apr 13 2018
I think this functionality can probably migrate to the build config service I'm working on.
,
Apr 16 2018
,
Apr 16 2018
One thing to keep in mind... Current branch util behavior is dependent on a lot of values in the manifest. We used to break it regularly, until branch-util tests were implemented and run on a regular basis (in the master-release builder). Those failures were hard to diagnose because of the gap in time between landing a manifest change and creating a new branch. I'm not saying we need the same solution, just that it's an issue to consider.
,
Apr 16 2018
Thanks, I'll keep an eye on that.
,
Jul 13
Issue 833494 has been merged into this issue.
,
Jul 23
Is this bug still the right place to talk about making a working branch utility for ToT?
,
Jul 23
Lann, this is related to, but different from build config. This is about actually creating the branches in ChromeOS git repositories.
,
Aug 15
Need to give this some attention. Will look at giving to new team member.
,
Sep 11
Any update on this?
,
Sep 11
Charles, I was thinking of giving this to Evan for his starter project. What do you think?
,
Sep 11
I have a different project.
,
Sep 11
Okay, Mike, as TL can you delegate to a different Build team member?
,
Sep 13
Don: are you aware of any docs for the branch-util-tryjob ? the only one i could find is: https://sites.google.com/a/google.com/chromeos/resources/engineering/releng/chrom-os-release-engineering-admin-faq iiuc, the reason we went with a branch-util trybot in the first place (issue 249566) was so that we could isolate ACLs and resources: TPMs didn't need push/branch creation access in their accounts, or a full CrOS checkout, they could simply kick off a remote trybot and it would do everything. but going by the documentation, TPMs have had to use --local since at least Dec 2013 which pretty much negates the point of doing it in a remote tryjob. so if there aren't any other advantages to doing it as a tryjob, looks like we should rip the branch_stages logic into a dedicated branch_util script that TPMs can run directly. if we wanted a remote tryjob, the stages logic could just as easily run the script. from what i can tell, the "only" thing the script needs to do is: - have the user specify the manifest version to branch - have the user specify the branch name - we load the manifest, process its directives, and create all the refs on the remote server is there anything else i'm missing here ?
,
Sep 13
I think you have it. We can't currently give builders enough permission to create branches without them ALWAYS having enough permissions to create branches and rewrite history, which was deemed a security hole. That wouldn't be a limitation if we took full advantage of vadimsh@'s service account work. Note: I once tried ripping the logic out of stages into a standalone script, but found them to be overly tied to cbuildbot. That created enough complexity that I couldn't finish it in the limited time I was willing to invest. Also note: lannm@'s new sync script will probably prove useful for this.
,
Sep 19
I think comment 25 is mostly right, there are a couple other options we want to support, like deleting or renaming branches which are occasionally useful to fix mistakes.
,
Sep 19
,
Sep 22
,
Sep 23
,
Sep 23
,
Sep 24
I'm being asked to write the new tool?
,
Oct 1
going to get lamont to work on this
,
Oct 9
,
Nov 26
Can we please get some traction here? I think it's important our branching tooling works w/o dragging around workarounds for months at a time. The original regression was reported back in March.
,
Nov 26
Evan. the long term solution for this will be integration into the build service. Can we take a short term look and see if we can fix this on tot currently.
,
Dec 3
,
Dec 11
How are things going here? Any updates to distill so we can determine where things are at?
,
Dec 11
This is a pretty important piece of our/any release pipeline - getting branches generated deterministically and correctly. I sure hope we can get some traction on this.
,
Dec 13
General update: I've been looking into how the current util works. Also spoke with dgarrett@ a bit about it. As shapiroc@ said, the long term goal is to merge this utility into the Build Service, which is currently about 2-3 days from running E2E. dgarrett@ suggested writing a stand-alone script to replace the tryjob as a temporary fix, but I'm hesitant to do this. Writing such a script would still take me a sizable chunk of time (maybe 1-2 weeks). Not to mention, the build service lives in google3 so I doubt we could reuse the script in any useful capacity. In my mind it seems more worthwhile to invest those 2 weeks into migrating the tool entirely to its final resting place. I estimate that this will ~3 work weeks to do properly. Happy to hear your thoughts. I'll try to be posting weekly updates from here on about the status of the project.
,
Dec 13
We do have a workaround for creating branches (per #4) so as long as that's not broken, I don't think we are in rush to get the new "feature" ready, IMHO.
,
Dec 17
Update: last week, the build team changed the scope of the build server, and consequently we've decided to replace the branch tryjob with a standalone script. You may find the rationale for not fixing the existing job here: go/branch-util-revamp The interface for the new tool is defined in this CL: https://chromium-review.googlesource.com/c/chromiumos/chromite/+/1378694 Please comment your concerns about the interface there. The present scope of the tool is for it to do *only* what the original tryjob could do. Additional features may be added but probably next quarter and at a lower priority than simply bringing branch util back to ToT.
,
Jan 6
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/0bf2c7eca8d4c76228482f4aadda9edc377629c3 commit 0bf2c7eca8d4c76228482f4aadda9edc377629c3 Author: Evan Hernandez <evanhernandez@chromium.org> Date: Sun Jan 06 04:08:27 2019 cros branch: Add cros branch command (no impl). This CL provides an avenue for discussing the new branch command interface. TEST=cros branch -h BUG=chromium:825241 Change-Id: Ifa14803bd193201f110b301781033a2ee105e14f Reviewed-on: https://chromium-review.googlesource.com/1378694 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: David McMahon <djmm@chromium.org> Reviewed-by: Bernie Thompson <bhthompson@chromium.org> [add] https://crrev.com/0bf2c7eca8d4c76228482f4aadda9edc377629c3/cli/cros/cros_branch.py
,
Jan 6
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/ed43a650f0c063e682bab0019ff786498c2347b6 commit ed43a650f0c063e682bab0019ff786498c2347b6 Author: Evan Hernandez <evanhernandez@chromium.org> Date: Sun Jan 06 04:08:28 2019 chromite: repo_manifest reads project annotations. TEST=./repo_manifest_unittest BUG=chromium:825241 Change-Id: I3c7a97941c4845c9c34cfe2255ead666b8525f85 Reviewed-on: https://chromium-review.googlesource.com/1384764 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Lann Martin <lannm@chromium.org> [modify] https://crrev.com/ed43a650f0c063e682bab0019ff786498c2347b6/lib/repo_manifest_unittest.py [modify] https://crrev.com/ed43a650f0c063e682bab0019ff786498c2347b6/lib/repo_manifest.py
,
Jan 8
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/af1fe10f99b95a23575cc9db6a30c0c1f26b1fa3 commit af1fe10f99b95a23575cc9db6a30c0c1f26b1fa3 Author: Evan Hernandez <evanhernandez@chromium.org> Date: Tue Jan 08 19:44:02 2019 cros branch: Locally fetch & branch off manifest. TEST=./run_tests cli/cros/cros_branch_unittest BUG=chromium:825241 Change-Id: Ic63165dfa9c6ea4ec765e55e4697b331333c4cc4 Reviewed-on: https://chromium-review.googlesource.com/1381062 Commit-Ready: Evan Hernandez <evanhernandez@chromium.org> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Jason Clinton <jclinton@chromium.org> [add] https://crrev.com/af1fe10f99b95a23575cc9db6a30c0c1f26b1fa3/cli/cros/cros_branch_unittest [modify] https://crrev.com/af1fe10f99b95a23575cc9db6a30c0c1f26b1fa3/cli/cros/cros_branch.py [add] https://crrev.com/af1fe10f99b95a23575cc9db6a30c0c1f26b1fa3/cli/cros/cros_branch_unittest.py
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/b2fb7ceaeff383de3565039b40a7e9143d7489e6 commit b2fb7ceaeff383de3565039b40a7e9143d7489e6 Author: Evan Hernandez <evanhernandez@chromium.org> Date: Thu Jan 10 21:58:30 2019 chromite: repo_manifest exposes includes. Rather than reimplementing `repo manifest`, this allows repo_manifest.Manifest to serve as a thin wrapper around manifest XML files. It will be necessary for repairing manifests in `cros branch`. TEST=./run_tests lib/repo_manifest_unittest BUG=chromium:825241 Change-Id: I6169556896ee56848b28562025091d5fe372eed5 Reviewed-on: https://chromium-review.googlesource.com/1387867 Commit-Ready: Evan Hernandez <evanhernandez@chromium.org> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Lann Martin <lannm@chromium.org> [modify] https://crrev.com/b2fb7ceaeff383de3565039b40a7e9143d7489e6/lib/repo_manifest_unittest.py [modify] https://crrev.com/b2fb7ceaeff383de3565039b40a7e9143d7489e6/lib/repo_manifest.py
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/3b5d9e029f3921428fa44445faa11b95fb0c893d commit 3b5d9e029f3921428fa44445faa11b95fb0c893d Author: Evan Hernandez <evanhernandez@chromium.org> Date: Thu Jan 10 21:58:31 2019 cros branch: Partially repair branched manifests. All branchable projects in the branched manifest must point to the new branch. The original tryjob did this manually (i.e., it was not automatically handled by CBuildBot). This CL begins to reimplement that functionality, relying heavily on the new standard lib for manifest manipulation, chromite/lib/repo_manifest. Right now, only project revision is repaired. Other repairs will be implemented in the next CL. TEST=./run_tests cli/cros/cros_branch_unittest BUG=chromium:825241 Change-Id: I36ca58c0f3eb2d08028823a40c335bfa34c20447 Reviewed-on: https://chromium-review.googlesource.com/1401643 Commit-Ready: Evan Hernandez <evanhernandez@chromium.org> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Evan Hernandez <evanhernandez@chromium.org> [modify] https://crrev.com/3b5d9e029f3921428fa44445faa11b95fb0c893d/cli/cros/cros_branch.py [modify] https://crrev.com/3b5d9e029f3921428fa44445faa11b95fb0c893d/cli/cros/cros_branch_unittest.py
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/f043718aa77e60aa66315b4f1f6f4b04df50b60a commit f043718aa77e60aa66315b4f1f6f4b04df50b60a Author: Evan Hernandez <evanhernandez@chromium.org> Date: Thu Jan 10 21:58:31 2019 cros_branch_unittest: Split tests. BranchCommandTest was beginning to look less like a unit test and more like a functional test. Split it up in anticipation of future CLs that add complexity. TEST=./run_tests cli/cros/cros_branch_unittest BUG=chromium:825241 Change-Id: Ibe52c9d0e45351ff5816114574ae506747358c4c Reviewed-on: https://chromium-review.googlesource.com/1401644 Commit-Ready: Evan Hernandez <evanhernandez@chromium.org> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Lann Martin <lannm@chromium.org> [modify] https://crrev.com/f043718aa77e60aa66315b4f1f6f4b04df50b60a/cli/cros/cros_branch_unittest.py
,
Jan 10
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/54cd0f1d2270f6f2468bf65faa60d5c1044812db commit 54cd0f1d2270f6f2468bf65faa60d5c1044812db Author: Evan Hernandez <evanhernandez@chromium.org> Date: Thu Jan 10 21:58:31 2019 cros branch: Split helpers for increased testing CanBranchProject in particular is core to cros branch and should be tested thoroughly. TEST=./run_tests cli/cros/cros_branch_unittest BUG=chromium:825241 Change-Id: I73aefbc546a6a1f361aec09c8532d5efe48b9f8b Reviewed-on: https://chromium-review.googlesource.com/1401645 Commit-Ready: Evan Hernandez <evanhernandez@chromium.org> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Jason Clinton <jclinton@chromium.org> Reviewed-by: Lann Martin <lannm@chromium.org> [modify] https://crrev.com/54cd0f1d2270f6f2468bf65faa60d5c1044812db/cli/cros/cros_branch.py [modify] https://crrev.com/54cd0f1d2270f6f2468bf65faa60d5c1044812db/cli/cros/cros_branch_unittest.py
,
Jan 11
I don't know if this has been covered. Can we add a requirement for the new branch-util to fail with an error if a repeated branch point is being specified. Possibly adding a --force type option, if you really intend to duplicate the build number. I will capture this in a separate bug. -Bob
,
Jan 11
,
Jan 11
Added crbug:921215 with a feature request. Thanks for your consideration.
,
Jan 15
The new tool is already configured to fail when the branch exists remotely. To overwrite the existing branch, you must specify --force. On ToT chromite you should be able to run `cros branch -h` to see the full interface. Update: - Remote branch creation is about 2 or 3 CLs away. Right now it can successfully create branches on local checkouts. My primary time sink for remote is writing a functional test. - Branch renaming and deletion should come shortly after or in parallel. They're much less complicated. Probably will be some delays because I was out sick yesterday and today, and next week I will be traveling for meetings. I'm shooting to be finished by end of next week if everything goes well, but it could stretch one more week.
,
Jan 15
To clarify the name of the branch does not have to be identical for it to break things, it merely has to share the version number it branched at with another branch. For example if someone generates a branch stabilize-1234.B then someone else comes in and creates release-1234.B then when either builds it will be conflicting with the other over the version space (e.g. is 1234.2.0 from the stabilize or the release?). The easiest way to do this would be to leverage the naming convention, don't allow the same two numbers to exist in the set of branches (this is what we do by hand today and we should check automatically to start IMO). However this relies on humans to ensure the branch names match so a more comprehensive way would be to check the manifest-versions repo for an existing version being used, though this also has a race in it if the other branch had not tried to build one yet.
,
Jan 15
Thanks for the clarification. Branch names are generated in the new tool. Correct me if I am wrong, but it seems sufficient to check if a branch exists with the tag -<major version>.B. If a branch off that version exists, the tool can fail unless --force is specified, in which case it will ask for confirmation. BTW, I thought minibranches increased the third version number, not the second?
,
Jan 15
Checking the version in the branch name is probably fine to start, if no one overrides the branch naming convention then this would be sufficiently reliable. For the version numbers it depends on where one branches from, if one branches from ToT XXXX.0.0 then one gets XXXX.Y.0, if one branches from XXXX.Y.0 one gets XXXX.Y.Z, one cannot branch XXXX.Y.Z (no 4th digit supported). The version numbers are illustrated a bit at https://screenshot.googleplex.com/JyEt65paw18
,
Jan 19
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/741cee1e3ff5941bdd5492479b277cb6992373db commit 741cee1e3ff5941bdd5492479b277cb6992373db Author: Evan Hernandez <evanhernandez@chromium.org> Date: Sat Jan 19 04:05:13 2019 cros branch: Completely repair manifest. Repairing a manfiest means setting the revision attribute of each project tag to point to the new branch (if a branch was indeed created for that project) or to point to the pinned revision (to ToT) if the project was tagged "pinned" (tagged "ToT"). The original branch-util optimized for manifest readability by preserving defaults. The new tool prefers code readability and explicitly sets project revisions, regardless of whether the project was branched, pinned, or left on ToT. TEST=./run_tests cli/cros/cros_branch_unittest BUG=chromium:825241 Change-Id: I62153c687a8bb148ee0e525642d95f3176721e04 Reviewed-on: https://chromium-review.googlesource.com/1403874 Commit-Ready: Evan Hernandez <evanhernandez@chromium.org> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Jason Clinton <jclinton@chromium.org> Reviewed-by: Don Garrett <dgarrett@chromium.org> [modify] https://crrev.com/741cee1e3ff5941bdd5492479b277cb6992373db/cli/cros/cros_branch.py [modify] https://crrev.com/741cee1e3ff5941bdd5492479b277cb6992373db/cli/cros/cros_branch_unittest.py
,
Jan 19
(4 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/chromite/+/74c8e8a018884141f76fe6295b57ea3a4eab850d commit 74c8e8a018884141f76fe6295b57ea3a4eab850d Author: Evan Hernandez <evanhernandez@chromium.org> Date: Sat Jan 19 04:05:13 2019 repo_manifest: _ManifestElement supports del. `cros branch` needs this to delete attributes during manifest repair after branching. TEST=./run_tests lib/repo_manifest_unittest BUG=chromium:825241 Change-Id: I6cde90e606ad3a4066888ffeaa66bccf73880a47 Reviewed-on: https://chromium-review.googlesource.com/1407821 Commit-Ready: Evan Hernandez <evanhernandez@chromium.org> Tested-by: Evan Hernandez <evanhernandez@chromium.org> Reviewed-by: Lann Martin <lannm@chromium.org> [modify] https://crrev.com/74c8e8a018884141f76fe6295b57ea3a4eab850d/lib/repo_manifest_unittest.py [modify] https://crrev.com/74c8e8a018884141f76fe6295b57ea3a4eab850d/lib/repo_manifest.py |
|||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||
Comment 1 by yueherngl@chromium.org
, Mar 23 2018