Follow-up API / in-tree check to ensure that the in-tree value now asserts the expected version passed in the payload.

Currently working on a script to examine the pushlog for recent version bump changes.

As a concept, rather than code review, what's your opinion? I'm thinking if we have a task that runs the following, then this should catch most error cases outside treescript: ./mach python ./testing/mozharness/scripts/release/verify_version_bump.py --since-revision ${GECKO_HEAD_REVISION}

I'm less sure on this one, in particular because it has mozilla-release hardcoded and because it does a check unbounded starting at HEAD. I'd like to maybe incorporate the tagging of BuildN we did, so that it checks between buildN and HEAD for a version bump, and noteworthy of course is that with the code as-is, a ver bump on a branch (that doesn't share an ancestor with our buildN) will still trigger this as ok. Since we have hg locally we could do (against our local hg clone) something like: $ hg log -q -r "(FIREFOX_62_0b5_BUILD1:beta and descendants(FIREFOX_62_0b5_BUILD1))" browser/config/version_display.txt browser/config/version.txt where `beta` would be the head rev. This uses `:` which basically means `anything pushed from the place the BUILDN tag is and our head` and then descendants ensures we get only things that are offshoot from buildN

(In reply to Justin Wood (:Callek) from comment #4) > I'm less sure on this one, in particular because it has mozilla-release > hardcoded and because it does a check unbounded starting at HEAD. This is why I asked about the concept, not for a code review :) The current draft I have doesn't have it hard-coded. So, are you, in principle, ok with the idea of a script like this existing, and being run a after the version bump task? > I'd like to maybe incorporate the tagging of BuildN we did, so that it > checks between buildN and HEAD for a version bump, What does this give us that's not covered by passing in the build revision? > and noteworthy of course > is that with the code as-is, a ver bump on a branch (that doesn't share an > ancestor with our buildN) will still trigger this as ok. I'm not sure I'm following. This is looking for a change to those files in a given repo, regardless of ancestry. > Since we have hg locally we could do (against our local hg clone) something > like: We discussed that this would be best in a separate task, so we don't have a clone locally at the point this would be run. We could add it, but it's heavyweight. > $ hg log -q -r "(FIREFOX_62_0b5_BUILD1:beta and > descendants(FIREFOX_62_0b5_BUILD1))" browser/config/version_display.txt > browser/config/version.txt > > where `beta` would be the head rev. This uses `:` which basically means > `anything pushed from the place the BUILDN tag is and our head` and then > descendants ensures we get only things that are offshoot from buildN If we're ok with doing a clone just to run this command, then sure. If we're doing that then the script probably shouldn't live inside mozharness as it does for now.

(In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #5) > (In reply to Justin Wood (:Callek) from comment #4) > > I'm less sure on this one, in particular because it has mozilla-release > > hardcoded and because it does a check unbounded starting at HEAD. > > This is why I asked about the concept, not for a code review :) The current > draft I have doesn't have it hard-coded. So, are you, in principle, ok with > the idea of a script like this existing, and being run a after the version > bump task? Yes, I'm ok with it, I'm not certain the talk in channel the other day felt this was even strictly necessary but I have no objections. > > I'd like to maybe incorporate the tagging of BuildN we did, so that it > > checks between buildN and HEAD for a version bump, > > What does this give us that's not covered by passing in the build revision? if we build on rev B then TB builds on rev C and ver bumps with rev D and we never ver bumped (E was pushed by a dev) we could think the TB bump was ours. A-B-E \-C-D > > and noteworthy of course > > is that with the code as-is, a ver bump on a branch (that doesn't share an > > ancestor with our buildN) will still trigger this as ok. > > I'm not sure I'm following. This is looking for a change to those files in a > given repo, regardless of ancestry. This hg command would make sure that we don't find a ver bump like this (C is our buildN tag) where a ver bump happened on E, though wasn't our release (our release would look at C, F, G, H) and we might have ver bumped with H but the HEAD of beta might be F. A - B - C - F | \- G - H \- D - E > > Since we have hg locally we could do (against our local hg clone) something > > like: > > We discussed that this would be best in a separate task, so we don't have a > clone locally at the point this would be run. We could add it, but it's > heavyweight. I think we're both confused then. I'm imagining this running as a seperate-from-treescript task, which clones hg, (ala: run-task) and then runs mozharness from that hg clone. This would mean we have an hg clone locally. Are you thinking differently? > > $ hg log -q -r "(FIREFOX_62_0b5_BUILD1:beta and > > descendants(FIREFOX_62_0b5_BUILD1))" browser/config/version_display.txt > > browser/config/version.txt > > > > where `beta` would be the head rev. This uses `:` which basically means > > `anything pushed from the place the BUILDN tag is and our head` and then > > descendants ensures we get only things that are offshoot from buildN > > If we're ok with doing a clone just to run this command, then sure. If we're > doing that then the script probably shouldn't live inside mozharness as it > does for now. If you're thinking differently (no hg clone at all) then I agree, no sense in using hg commands directly (more overhead), but if we already have a clone due to this then using hg commands is probably better.

(In reply to Justin Wood (:Callek) from comment #6) > (In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #5) > > (In reply to Justin Wood (:Callek) from comment #4) > > > I'm less sure on this one, in particular because it has mozilla-release > > > hardcoded and because it does a check unbounded starting at HEAD. > > > > This is why I asked about the concept, not for a code review :) The current > > draft I have doesn't have it hard-coded. So, are you, in principle, ok with > > the idea of a script like this existing, and being run a after the version > > bump task? > > Yes, I'm ok with it, I'm not certain the talk in channel the other day felt > this was even strictly necessary but I have no objections. "We don't trust external services" is the core part of the request that we've been given, for this sort of graph testing. I don't think I managed to get that across, last week, as people thought 'hg push' could just be trusted to do the right thing. > > > I'd like to maybe incorporate the tagging of BuildN we did, so that it > > > checks between buildN and HEAD for a version bump, > > > > What does this give us that's not covered by passing in the build revision? > > if we build on rev B then TB builds on rev C and ver bumps with rev D and we > never ver bumped (E was pushed by a dev) we could think the TB bump was ours. > > A-B-E > \-C-D Thunderbird builds from mozilla-beta, mozilla-release and mozilla-esr60? I thought they had comm-* repositories? https://hg.mozilla.org/mozilla-central/pushlog/ only appears to have mozilla-central pushes in it, similarly for the others. What am I missing? > > We discussed that this would be best in a separate task, so we don't have a > > clone locally at the point this would be run. We could add it, but it's > > heavyweight. > > I think we're both confused then. I'm imagining this running as a > seperate-from-treescript task, which clones hg, (ala: run-task) and then > runs mozharness from that hg clone. This would mean we have an hg clone > locally. > > Are you thinking differently? The impression I got was that if it was going to have 'clone a repo for a few minutes, just to tick the box' then it should just be in treescript, as it would take less time. It does seem remarkably heavyweight to do a clone just for one 'hg log' command, although I do see that using the tags makes the query cleaner (although the hg command itself, not so much) > > > $ hg log -q -r "(FIREFOX_62_0b5_BUILD1:beta and > > > descendants(FIREFOX_62_0b5_BUILD1))" browser/config/version_display.txt > > > browser/config/version.txt > > > > > > where `beta` would be the head rev. This uses `:` which basically means > > > `anything pushed from the place the BUILDN tag is and our head` and then > > > descendants ensures we get only things that are offshoot from buildN Why is 'beta' the head revision here? Or is that just the example of using mozilla-beta?

I think that due to disagreements over whether this is required or not, and the apparent complexity involved in making sure it's safe against the various odd branching/merging practices in use, it's probably not worth the effort at this stage.