Ticket #180 (closed enhancement: fixed)

Opened 5 years ago

Last modified 10 months ago

buildbot try does the wrong thing with mercurial queues

Reported by: dsallings Owned by: dsallings
Priority: major Milestone: 0.8.+
Version: 0.7.6 Keywords: hg
Cc: afri, dustin, dsallings, marcusl, burcin@…, malcolm.parsons@…

Description

We'd like to be able to use buildbot try and mercurial queues. It's a pretty simple change to make:

Instead of asking for the tree identity, ask for the identity of the qparent. If there's not a qparent, ask for the current identity (as is done now). Then always diff against the discovered patch level.

A patch is attached that seems to do the job well.

Attachments

try-mq.diff Download (792 bytes) - added by dsallings 5 years ago.
try with mq support
180-testcase.diff Download (3.2 KB) - added by Ben 5 years ago.
draft of testcase
test.log Download (2.9 KB) - added by dustin 4 years ago.
did-not-see-expected-test.log

Change History

Changed 5 years ago by dsallings

try with mq support

comment:1 Changed 5 years ago by afri

  • Cc afri added

comment:2 Changed 5 years ago by Ben

  • Cc dustin added

Seems reasonable to me, any objections to that one ?

For the non Mercurial aware, this patch means that if you have any pending patch (as opposed to normal changeset) on your repository, the diff is done with those actual pending patches. If not, revert back to the current modif on the working dir.

comment:3 Changed 5 years ago by dustin

  • Status changed from new to closed
  • Resolution set to fixed

 http://repo.or.cz/w/buildbot.git?a=shortlog;h=refs/remotes/dustin/bug180

(note: I have no way to test this, as I don't use hg -- I'm going on Ben's "seems reasonable")

comment:4 Changed 5 years ago by Ben

  • Status changed from closed to reopened
  • Resolution fixed deleted

Note that a test case would be nice ...

I'll try to combine one tonight.

comment:5 Changed 5 years ago by Ben

  • Owner set to dsallings
  • Status changed from reopened to new

I cannot make my testsuite working ...

So I leave the ticket open, untill *someone* fix the testsuite or the patch ....

Here is where I am so far (see patch actually)

Changed 5 years ago by Ben

draft of testcase

comment:6 Changed 5 years ago by Ben

  • Type changed from defect to enhancement

to finish for today, it's more a wish than a defect ...

comment:7 Changed 5 years ago by dustin

  • Keywords review added; mercurial try removed

comment:8 Changed 5 years ago by dustin

  • Cc dsallings added
  • Keywords review removed

OK, this patch is available in integrated form at:

 http://repo.or.cz/w/buildbot.git?a=shortlog;h=refs/remotes/dustin/bug180

Since this is a "draft", I haven't pulled it into the dev tree yet, and will un-mark it for review.

===============================================================================
[ERROR]: buildbot.test.test_vc.Mercurial.testTryMq

Traceback (most recent call last):
  File "/home/dustin/devel/projects/buildbot/t/buildbot/buildbot/test/test_vc.py", line 345, in check
    + ": see logs for stdout")
exceptions.RuntimeError: command ['/usr/bin/hg', 'qnew', '-f', 'patch.diff'] finished with exit code 255: see logs for stdout

By the way, dsallings, what is your full name and email address, so I can give you credit?

comment:9 Changed 5 years ago by dsallings

Great, thanks. Dustin Sallings <dustin@…>

comment:10 Changed 5 years ago by dsallings

I just realized this was showing an error. I haven't used hg with buildbot in a while, but I was using it with this recipe. Is it actually failing? If so, does the log show anything?

comment:11 Changed 4 years ago by dustin

looks like 'qnew' is too new for my version of hg?

[I] dev-util/mercurial
     Available versions:  1.0.1-r2 ~1.0.1-r3 1.0.2 {bash-completion bugzilla cvs darcs emacs git gpg subversion test zsh-completion}
     Installed versions:  1.0.2(02:02:29 12/03/08)(-bash-completion -bugzilla -emacs -gpg -test -zsh-completion)
     Homepage:            http://www.selenic.com/mercurial/
     Description:         Scalable distributed SCM

comment:12 Changed 4 years ago by dsallings

You have to enable mq before the commands work:

~/.hgrc:

[extensions] hgext.mq =

comment:13 Changed 4 years ago by dsallings

Sorry, poorly formatted.

[extensions]
hgext.mq=

comment:14 Changed 4 years ago by dustin

Hmm, I don't really know what that means, but is there some way to detect that and either skip the tests or give a helpful failure message in that case? That might deserve another bug.

For now, I see this, which I don't really know how to diagnose.

===============================================================================
[FAIL]: buildbot.test.test_vc.Mercurial.testTryMq

Traceback (most recent call last):
  File "/home/dustin/devel/projects/buildbot/t/buildbot/buildbot/test/test_vc.py", line 964, in _do_getpatch_trunkhead_2
    self.failUnlessIn("try",ss.patch[1])
  File "/home/dustin/devel/projects/buildbot/t/buildbot/buildbot/test/test_vc.py", line 382, in failUnlessIn
    self.failUnless(string.find(substring) != -1, msg)
twisted.trial.unittest.FailTest: did not see the expected substring 'try' in string ''
-------------------------------------------------------------------------------
Ran 1 tests in 4.782s

By the way, I have cloned the buildbot repo at  http://github.com/djmitche/buildbot. If you want to meet me in #buildbot (freenode), maybe we can figure out a way to make this patch process easier.

comment:15 Changed 4 years ago by dsallings

hg help | grep qnew

comment:16 Changed 4 years ago by dustin

OK, I added a fix to detect the mq extension and skip if it's not present, at

 http://github.com/djmitche/buildbot/tree/bug180

However, with the proper .hgrc, this is still failing..

comment:17 Changed 4 years ago by dustin

hmm, this may be due to an earlier commit. Running 'git bisect' now.

comment:18 Changed 4 years ago by dsallings

This is just under a year old. I pretty much don't use hg anymore for anything, but the functionality was very useful when I was.

That is to say, I don't know how useful I'll be in getting it up-to-date, as I don't use it anymore, and last time I tried updating local git changes over cvs, there were too many conflicts.

It might be good to reach out to more hg users and see if anyone can at least tell if it still works.

Changed 4 years ago by dustin

did-not-see-expected-test.log

comment:19 Changed 4 years ago by dustin

gotcha, thanks.

comment:20 Changed 4 years ago by marcusl

  • Cc marcusl added

comment:21 Changed 4 years ago by marcusl

Is it skipping the entire Mercurial tests or just the mq-part? It looks like it's the entire shebang, which is a bit worrying, no?

Also, how about using the repository's hgrc file ( i.e .hg/hgrc) to just enable the extension for our test repo?

Just my 0.02$.

comment:22 Changed 4 years ago by dustin

  • Milestone changed from undecided to 0.7.+

This is still failing for me; I just rebased it onto master.

 http://github.com/djmitche/buildbot/tree/bug180

comment:23 Changed 4 years ago by marcusl

I've merged with current master and made a small mod that only skips the tryMqTest. It fails here too, but at least the failure is more localized and more test work without mq-enabled.

 http://github.com/marcusl/buildbot/tree/bug180

comment:24 Changed 4 years ago by dustin

I've got your most recent changes merged in my bug180, but it's still failing. Just FYI :)

comment:25 Changed 3 years ago by dustin

  • Milestone changed from 0.8.+ to 0.7.12

What's going on here? I pulled marcusl's bug180 branch. I have

[extensions]
hgext.mq=

in my .hgrc, and

dustin@euclid ~/code/buildbot/t/buildbot [bug180] $ hg showconfig | grep hgext
extensions.hgext.mq=

yet

[SKIPPED]: buildbot.test.test_vc.MercurialInRepo.testCheckout

Add 'hgext.mq =' to the '[extensions]' section of ~/.hgrc

We should really either get this merged or give up..

comment:26 Changed 3 years ago by marcusl

Haven't worked on for.. well.. 8 months.

I'm not using mq either, so I've never gotten around to scratch this itch properly.

comment:27 Changed 3 years ago by dustin

  • Milestone changed from 0.7.12 to 0.8.0

next release, then :)

comment:28 Changed 3 years ago by dustin

  • Milestone changed from 0.8.0 to 0.8.+

bump..

comment:29 Changed 3 years ago by dustin

  • Keywords hg added

comment:30 Changed 19 months ago by burcin

  • Cc burcin@… added

attachment:try-mq.diff Download works for me, after manually applying the patch to tryclient.py. Could this be merged for the next release?

The first hunk of  https://github.com/marcusl/buildbot/commit/50c3be07aa646a3668e38503e3abd25d1bb00879 seems to remove the code that checks if mq is available. Perhaps reverting this would fix the tests.

comment:31 Changed 19 months ago by marcusl

The commit was made to only disable the testTryMq() test, not the entire Mercurial test case, when mq is not enabled. (Since I and many others didn't use MQ, Mercurial tests weren't run at all.)

That was the intention at least. Looking at it now, it seems as if MercurialHelper?._qnew_supported might be set (or not), and the commit removed some code which would update that value again.

The right thing would be to make sure that _qnew_supported has a correct value always, and not to rely on the unit tests to check and set it. (Could be a property that's set lazily inside MercurialHelper?, on first access....)

comment:32 Changed 11 months ago by Malcolm Parsons

  • Status changed from new to closed
  • Resolution set to fixed

try: For mercurial, create a patch containing all outgoing changesets on the current branch. (fixes #865, #180)

Changeset: d867a203b2cf613422a0971f5e31789c9259c339

comment:33 Changed 11 months ago by Dustin J. Mitchell

Merge branch 'mercurialtry' of  git://github.com/pepsiman/buildbot

  • 'mercurialtry' of  git://github.com/pepsiman/buildbot: try: For mercurial, create a patch containing all outgoing changesets on the current branch. (fixes #865, #180) Mercurial: Remove any added files before running update --clean Mercurial: Support applying a patch for Try builds Changeset: 5aee01d01da16de441ccd43af5806f1db087ed91

comment:34 Changed 11 months ago by Malcolm Parsons

try: For mercurial, create a patch containing all outgoing changesets on the current branch. (fixes #865, #180)

Changeset: d867a203b2cf613422a0971f5e31789c9259c339

comment:35 Changed 11 months ago by Dustin J. Mitchell

Merge branch 'mercurialtry' of  git://github.com/pepsiman/buildbot

  • 'mercurialtry' of  git://github.com/pepsiman/buildbot: try: For mercurial, create a patch containing all outgoing changesets on the current branch. (fixes #865, #180) Mercurial: Remove any added files before running update --clean Mercurial: Support applying a patch for Try builds Changeset: 5aee01d01da16de441ccd43af5806f1db087ed91

comment:36 Changed 11 months ago by Dustin J. Mitchell

Merge branch 'master' into nine

  • master: (80 commits) Force defaultBranch if repoURL is set. tabs to spaces upgrade to use the new getProcessOutput expectations framework fix pyflakes Doc fixes try: For mercurial, create a patch containing all outgoing changesets on the current branch. (fixes #865, #180) Mercurial: Remove any added files before running update --clean Mercurial: Support applying a patch for Try builds Add some test to doc and release-notes about HTPasswdAprAuth. Fix master-side SVN step. skip test_test_util_gpo on py27+tw900 Updated a partly wrong docstring in hgpoller Moved os.environment change to setUp/tearDown in hgpoller tests Move libaprutil password checking to an new subclass Skip instead of hide md5 and sha tests if libaprutil is missing. Use ctypes.util.find_library to resolve library name. Remove old GetProcessOutputMixin?. Fix gitpoller tests of environment passing. Fix p4 poller tests to use new gpo mixin. Fix svn poller tests to use new gpo mixin. ...

Conflicts:

master/buildbot/test/unit/test_changes_gitpoller.py master/buildbot/test/unit/test_changes_p4poller.py master/buildbot/test/unit/test_changes_svnpoller.py master/docs/manual/cfg-global.rst

Semantic Conflicts:

master/buildbot/changes/hgpoller.py + tests updated to use data API

Changeset: ece5f76ae1953974684f5573a3718e35c1864d9e

comment:37 Changed 10 months ago by pepsiman

  • Cc malcolm.parsons@… added
Note: See TracTickets for help on using tickets.