--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
You can see the information on codereview new UI (go to setting and unselect the "Deprecated UI" checkbox). I don't know if there are any other usage yet, but I'm hoping this will be used to allow sending CL with dependencies to the try bots.
I just got a strange variation of this:The current branch (kerberos) is tracking a local branch (master) with an open CL.Adding https://codereview.chromium.org/1151773003/#ps1 as a dependency patchset.Strange both because I am not changing master (it is identical to origin/master) and because https://codereview.chromium.org/1151773003 is neither open (it has landed) nor mine.
On Wed, Jun 24, 2015 at 2:30 PM, Anthony Berent <abe...@chromium.org> wrote:I just got a strange variation of this:The current branch (kerberos) is tracking a local branch (master) with an open CL.Adding https://codereview.chromium.org/1151773003/#ps1 as a dependency patchset.Strange both because I am not changing master (it is identical to origin/master) and because https://codereview.chromium.org/1151773003 is neither open (it has landed) nor mine.
-- Sylvain
For what it's worth, "master" on all my checkouts gets associated with some issue at some point, due to me accidentally running `git cl patch` while on master. Does it ever make sense to associate master with a CL? If not, maybe that branch could be special-cased?
For what it's worth, "master" on all my checkouts gets associated with some issue at some point, due to me accidentally running `git cl patch` while on master. Does it ever make sense to associate master with a CL? If not, maybe that branch could be special-cased?
On Fri, Jun 26, 2015 at 3:12 AM, Nico Weber <tha...@chromium.org> wrote:For what it's worth, "master" on all my checkouts gets associated with some issue at some point, due to me accidentally running `git cl patch` while on master. Does it ever make sense to associate master with a CL? If not, maybe that branch could be special-cased?Hiccups like these are exactly the reason why I do "git branch -D master" first thing after I create a new checkout :)
On Fri, Jun 26, 2015 at 12:12 AM, Mattias Nissler <mnis...@chromium.org> wrote:On Fri, Jun 26, 2015 at 3:12 AM, Nico Weber <tha...@chromium.org> wrote:For what it's worth, "master" on all my checkouts gets associated with some issue at some point, due to me accidentally running `git cl patch` while on master. Does it ever make sense to associate master with a CL? If not, maybe that branch could be special-cased?Hiccups like these are exactly the reason why I do "git branch -D master" first thing after I create a new checkout :)+1, could we make `fetch chromium` not end up with a "master" or does something need it?
My understanding is that a local master would only start to be a problem if it had patches associated with it. Don't do that?
On Fri, Jun 26, 2015 at 10:06 PM, Dirk Pranke <dpr...@chromium.org> wrote:My understanding is that a local master would only start to be a problem if it had patches associated with it. Don't do that?Well, the git cl implementation assumes that if the current branch has a local upstream branch which has a CL number attached, that CL must be a change the CL on the current branch depends on. Maybe that's too strong of an assumption? I might have set the upstream pointer to simplify rebasing etc., but the CLs may actually not have code dependencies so I don't want git cl to pick up upstream as a dependency. Or I might prefer to keep upstream pointing to origin/master even though I have dependent open CLs as commits on the current branch, in which case git cl has no chance to pick up the correct dependencies.
It may make sense to have a flag to override git cl's automatic dependency detection?
FWIW, gerrit solves all this by putting a Change-Id: line into the commit message, which allows it to find dependencies by looking at commit messages of previous commits on the current branch.
On Fri, Jun 26, 2015 at 1:22 PM, Mattias Nissler <mnis...@chromium.org> wrote:On Fri, Jun 26, 2015 at 10:06 PM, Dirk Pranke <dpr...@chromium.org> wrote:My understanding is that a local master would only start to be a problem if it had patches associated with it. Don't do that?Well, the git cl implementation assumes that if the current branch has a local upstream branch which has a CL number attached, that CL must be a change the CL on the current branch depends on. Maybe that's too strong of an assumption? I might have set the upstream pointer to simplify rebasing etc., but the CLs may actually not have code dependencies so I don't want git cl to pick up upstream as a dependency. Or I might prefer to keep upstream pointing to origin/master even though I have dependent open CLs as commits on the current branch, in which case git cl has no chance to pick up the correct dependencies.Okay, but neither of those things contradicts what I wrote or makes a local master useless with the current implementation, right? (Just making sure we're on the same page).
It may make sense to have a flag to override git cl's automatic dependency detection?I could agree that that might make sense.FWIW, gerrit solves all this by putting a Change-Id: line into the commit message, which allows it to find dependencies by looking at commit messages of previous commits on the current branch.I'm not familiar w/ how Gerrit solves this, but AFAICT Gerrit's model for branches and commits is quite different from Rietveld and it's not clear to me how we would map things over.
Note that you can branch off of origin/master instead of master to avoid this problem. That's precisely what `git new-branch` does (see `man depot_tools_tutorial` or open depot_tools/man/html/depot_tools_tutorial.html for an idea of how the flow using these tools looks, or `git new-branch --help` directly). If you use the recommended flow, you can always see what the tools' idea of the world looks like by running `git map-branches -vvv`.
So to recap, there are already multiple ways of avoiding having a stale/incorrect master dependency:* delete local master, use origin/master to view upstream HEAD and branch directly off origin/master or other feature branches for feature work (recommended)* have a local master, and ensure that you never start a CL off it (or run `git cl issue 0` to remove the issue from it), branch from that (or other features) to do feature work* have a local master, sometimes have a CL on it, but branch off origin/master (or other features) for new features
If you have a branch master (with a CL) and then another branch foo (with a CL), you're telling git (and the tools in depot_tools): "I have a series of changes called master (based on origin/master), and then another series of changes depending on it called foo". I don't think it makes sense to add a flag to the tool to tell it to ignore master in this case (i.e. "even though I told you I depend on that thing, pretend I don't").It may make sense to add a flag (or a git config option) which says "Don't upload any dependency information" for alternate branch layouts, but every option we add combinatorially adds to the maintenance burden of the tools overall. I'm not completely opposed to having a single "don't do anything fancy, I'm not using the recommended flow so my branches don't mean what you think they do" flag which all the tools use before they try to infer structure from the branch/commit layout though... at least in that world the recommended flow could gain new features without breaking people who have opted into their own thing (Ravi's change here would be an example of that). Our team isn't very big, and we don't get a lot of contributions from outside the team (but we're extremely grateful for the ones we do!), so more options hurts our ability to ensure that the entire toolchain works well with itself (self-consistency).I think adding another way to tell the tools to ignore the content of master is appealing mainly for the reason of "changing workflow is annoying"[1], but I don't think in the grand scheme of things it's really good for the tools (higher maintenance), for chromium overall (adds complexity for every developer, current and incoming. `git cl upload --help` already takes an alarming amount of screen real estate), or necessarily even for those asking for the flag (only you can be the judge of that, but such a new flag would probably be prone to breakage or misinterpretation over time because the high degree of interdependency between the tools which assume the repo to be in a certain state).
On Sat, Jun 27, 2015 at 10:41 AM, Robert Iannucci <iann...@chromium.org> wrote:Note that you can branch off of origin/master instead of master to avoid this problem. That's precisely what `git new-branch` does (see `man depot_tools_tutorial` or open depot_tools/man/html/depot_tools_tutorial.html for an idea of how the flow using these tools looks, or `git new-branch --help` directly). If you use the recommended flow, you can always see what the tools' idea of the world looks like by running `git map-branches -vvv`.
So to recap, there are already multiple ways of avoiding having a stale/incorrect master dependency:* delete local master, use origin/master to view upstream HEAD and branch directly off origin/master or other feature branches for feature work (recommended)* have a local master, and ensure that you never start a CL off it (or run `git cl issue 0` to remove the issue from it), branch from that (or other features) to do feature work* have a local master, sometimes have a CL on it, but branch off origin/master (or other features) for new featuresNeither of those match my workflow.I don't know git well. My (incomplete, incorrect, simplified) view of the world is that master is the upstream code (unless I screw something up – when things look off, I run `git reset --hard origin/master`) and my local branches are my changes.If I had no local master branch, I wouldn't know how to sync (I run `git checkout master; git pull` for that). I always branch off master for working on features. master is associated with some random CL because of me not being careful in every of my checkouts that I've checked so far (on this machine, it's apparently https://codereview.chromium.org/429793006).
Updates
There is now a way to skip dependency checks and uploads for specific local branches by running (using "test" as a branch name):
git config branch.test.skip-deps-uploads True
To undo the skipping:
git config --unset branch.test.skip-deps-uploads
To do the above commands globally (across all checkouts):
git config --global branch.test.skip-deps-uploads True
git config --global --unset branch.test.skip-deps-uploads
Next steps
The CQ will start rejecting CLs with open patchset dependencies. I plan to turn this on tomorrow.
Similar to the other CQ checks, you will be able to skip the open dependency check by using "NO_DEPENDENCY_CHECKS=true" in your CL’s description. Open dependency checks will be automatically skipped for dry runs.
After this check is in place in the CQ, apply_issue.py will be modified to apply all dependencies of a patchset. This will allow trybots to successfully test your partial diff uploaded due to tracked local branches (crbug/480453).
To recap, there will be two ways to skip the dependency checks:
* Use the git config settings mentioned above to skip dependency uploads of specific branches (eg: master). This is already live.
* Use the new keyword "NO_DEPENDENCY_CHECKS=true" to skip the CQ open dependency check on a per CL basis. This will be live tomorrow.