Add ability to trigger on more than one svn path

[jeberger@…]

Description

It would be nice to add the ability to trigger a build based on more than one svn path. The build would then use the most recent revision in which at least one of the watched paths was modified.

Rationale

We're using svn+Trac+Bitten to build Linux-based firmwares for our products. Our system is based on a solution similar to the Gentoo emerge system:

• There is a set of package descriptors that describe dependencies and build recipes. Every developer has a complete copy of all the descriptors on their computers;
• And there is a source repository where the actual package sources reside. When a developper wants to work on a given package, he only needs to check out the source for this package alone.

Our svn repository is therefore separated into two distinct repositories (actually, two distinct trees in the same repository):

• A repository for the sources;
• A repository for the package descriptors.

At present, Bitten triggers a rebuild when the sources are modified, but it ignores changes to the package descriptors. It would be nice if we could have Bitten watch both the path to the source and the path to the descriptors (an possibly the paths to the more important dependencies) and trigger a build whenever one of them changes.

[anonymous]

I created a patch that monitors multiple paths and triggers a build whenever any of those paths are modified. A discussion of the problem is on the newsgroup here: ​http://groups.google.com/group/bitten/browse_thread/thread/c61b0e25a0bcf4ba

[anonymous]

Patch adds support for triggering builds from multiple paths

[ebray]

Slightly updated version of this patch

[ebray]

Replying to anonymous:

I created a patch that monitors multiple paths and triggers a build whenever any of those paths are modified. A discussion of the problem is on the newsgroup here: ​http://groups.google.com/group/bitten/browse_thread/thread/c61b0e25a0bcf4ba

Sorry it took me so long to get back to you. I've actually been sitting on my comments for a couple weeks, but have just been too distracted by other things and kept forgetting to write them down.

I posted a slightly updated version of the patch that mostly just corrects a few Python noob-isms. I also removed the name change of the 'path' column in the build_config table to 'paths', as it was unaccompanied by a database schema update/migration script, and also because there's little to gain from a mere name change anyways.

I still couldn't recommend this patch just yet though due to two main problems:

1. The big for path in config.paths loop placed around most of collect_changes() in bitten.queue could lead to the same build being added to the queue multiple times. For example, if I have a config that triggers on path_a and path_b, and I have three revisions r1, r2, and r3 that all apply to both path_a and path_b, collect_changes() will yield the Build for each revision twice, in the order: r3, r2, r1, r3, r2, r1. So collect_changes() needs to be refactored somehow so that a build for each revision is only triggered once (per-platform/configuration), regardless of which path triggered it.
2. I'm still feeling like it's probably not a good idea to store multiple paths as a newline-separated list in the DB. Maybe I'm wrong, but it just seems like that's asking for trouble. Better to move Build Configuration-to-path mappings in a separate table, which of course would necessitate a schema update.

[anonymous]

+1 love this patch for building projects with externals

[dfraser]

Replying to ebray:

1. The big for path in config.paths loop placed around most of collect_changes() in bitten.queue could lead to the same build being added to the queue multiple times. For example, if I have a config that triggers on path_a and path_b, and I have three revisions r1, r2, and r3 that all apply to both path_a and path_b, collect_changes() will yield the Build for each revision twice, in the order: r3, r2, r1, r3, r2, r1. So collect_changes() needs to be refactored somehow so that a build for each revision is only triggered once (per-platform/configuration), regardless of which path triggered it.

That could be quite easily done by simply using the loop to generate a list of unique revisions, sorting them, and repeating...

1. I'm still feeling like it's probably not a good idea to store multiple paths as a newline-separated list in the DB. Maybe I'm wrong, but it just seems like that's asking for trouble. Better to move Build Configuration-to-path mappings in a separate table, which of course would necessitate a schema update.

Agreed. I think it would be worthwhile to do the schema change. Let's rather get the schema flexible enough now than have to do this sort of thing later and/or have hacks like newline-separated lists in the DB.

[sahendrickson@…]

Updated to address some issues and fix additional bugs

[sahendrickson@…]

I've updated the patch in the following ways:

• to address the issue of duplicate results from collect_changes()
• fixed the Source File Link Formatter?
• to address additional areas that were not updated

Issues still remaining:

• It doesn't update the database, I leave that for someone more experienced
• It doesn't update the tests.* files, I don't use these and can't really test changes

Thanks for the updates ebray and dfraser!

[anonymous]

First i would thanks you all here for this patch only it does not work for me. This patch does not work for externals outside of the project repository!

externals outside of the project repository starts with: "​http://", "​svn://" etc.

Could this also please be addressed??

[osimons]

#290 closed as duplicate.

[anonymous]

Replying to anonymous:

First i would thanks you all here for this patch only it does not work for me. This patch does not work for externals outside of the project repository!

externals outside of the project repository starts with: "​http://", "​svn://" etc.

Could this also please be addressed??

I'm not sure that I understand the issue. Could you clarify a little more, please? What specifically doesn't work for externals?

[osimons]

See also #302, and the possible need for a configuration to remember original paths and support rebuilding etc.

[sahendrickson@…]

This latest patch addresses the issues with "​http://" and "​svn://" addressed in the previous comment. It also recognizes java line numbers and revision numbers at the end of paths in the form of "path#revision". It also adds a link to the specific build revision of each path in the form of an appended "@revision" at the end of the resources. Finally, this patch works with trunk.

Critical issues that remain:

• There are four tests that fail. I cannot figure out why. Mainly this is due to the fact that I'm not that familiar with python and cannot figure out what method calls are occurring.

Minor issues that remain:

• The patch doesn't update the database. I leave that for someone more experienced to do this and it's not entirely necessary to do so. I only ask that the ultimate result leave the UI as is so that the user edits a list in a box. I really would not be happy with a +/- box for which paths can be added/removed. Specifically, I would not want the UI to function like the list of required properties for each slave in the build configuration. But, that's my own personal preference.
• The reports/*.py files assume that all files exist on the first path. I don't use any reports and have no way of testing the result. Probably a simple fix would be to iterate through the list of config.paths and use the first path for which the file exists.

Please, please, please, can someone fix the four failing tests?

For the minor issues, I suggest that this patch be integrated, and two new tickets be opened since these issues do not hinder any current (i.e., single path) configurations from functioning properly. I would be very grateful !!! :)

This patch is hard to update after each new commit to trunk since its changes span many different files and it really does affect a core assumption that currently exists regarding the number of paths. So, please, get this in as soon as possible so that mono-path and multi-path support does not further diverge :)

Thank you!

[sahendrickson@…]

Updates to fix url links in logs, assume first path in reports, passes all but a few tests

[osimons]

Thanks for the patch! Complicated stuff this, and I need some time to understand and comprehend all the consequences... I'll take at look at it.

[sahendrickson@…]

Updates to fix url links in logs again

[sahendrickson@…]

Another possibility, from the newsgroup, is to trigger builds too often, and instead enhance the slave to cancel a build if a change does not actually warrant it.

I like this idea, but would like being able to specify paths as well to scope the trigger of the build and because the tools I use on the slave side currently don't have any way of checking for whether a change was relevant or not.

[Mat Booth <trac@…>]

Replying to sahendrickson@…:

Another possibility, from the newsgroup, is to trigger builds too often, and instead enhance the slave to cancel a build if a change does not actually warrant it.

I like this idea, but would like being able to specify paths as well to scope the trigger of the build and because the tools I use on the slave side currently don't have any way of checking for whether a change was relevant or not.

What would be really cool, is recipe dependencies. Say I have a library and a bunch of apps that use that library all in source control. If someone makes a change to one of the apps, then Bitten would only build the app, but if someone makes a change to the library then Bitten would build the library and all the recipes that depend on it (all of the apps).

This would save us hours of build time by only building what needed rebuilding. At the moment we build absolutely every product whether it needs it or not.

[osimons]

More flexible recipe handling could be one way to solve this - which is also the subject of some other tickets (varying degrees of relevance): #90, #143, #230, #301.

One possibility is to add some rudimentary expressions to recipes, first and foremost a conditional expression perhaps at step-level to evaluate if this step should be performed:

<step id="two" if="family=='nt'">
    <!-- perform some Windows custom stuff here -->
</step>

That would likely need to be combined with some various other open tickets that request more variables available in templates (user, revision author, environment variables), and we could also make some new kinds of variables that make no sense in a pure substitution-context. One good example is a files collection containing all paths changed as part of the triggering changeset:

<step id="two" if="files.any.matches('^/subpathA') and author.matches('simon')">
    <!-- do something special that isn't always needed -->
</step>

I haven't actually put any thought into this - it is just an idea...

[sahendrickson@…]

I'd like to resolve this issue, but don't want to work on a solution unless it's going to get incorporated (too much time wasted last time). So, what's the consensus here at this point?

[shendric@…]

Removing my other e-mail address (hopefully the comment will prevent the submission as being marked spam?)

[Joseph.Robertson@…]

I support the recipe dependancies idea. Our project tree is split by module, most modules also being stand alone apps, and a top level module that integrates everything into a linux os filesystem (its very similar to uClinux). So if someone updates a app, the top level svn folder for the integrator is never disturbed and a build never happens.

Dependancies also need to support multiple repositories, our latest project is spun off into a new repository, but uses some apps from the original repository (2 different repos).

In the meantime I will try the patch here, many thanks for that.

Thanks.

[mikael@…]

Slightly re-factored and re-application of previous patch

[anonymous]

I was about to try this​ out on my own system, but the latest patch did not cleanly apply to my system, so I "manually" rolled the patch into trunk. Halfway through I realized that this won't solve my specific scenario (monitoring several paths in SVN, triggering the build on a different one) completely, so I have not actually tested this patch myself. Either way, it could work, it could also not work, try yourselves. Patch should apply agains r816.

[cboos]

A few comments on the form of the proposed code: lacking a specific style guide for Bitten, I suppose/hope the ​Trac:TracDev/CodingStyle applies... in which case things like the following:

 return Markup(tag.a(m.group(0), href=link)) +tag.i(Markup("@") + Markup(tag.a(build.rev, href=link+'?rev='+build.rev))) 

are problematic on several accounts (line too long, missing space around operators).

And for that specific line, you could probably do away with:

 return Markup(tag(tag.a(...),
                   tag.i("@", tag.a(build.rev, 
                                    href=link + '?rev=' + build.rev)))))

Nitpicking perhaps, but I hope you get the idea to improve the patch ;-)

[mikael@…]

patch against r816 (fixed indentation)

[mikael@…]

..

Nitpicking perhaps, but I hope you get the idea to improve the patch ;-)

Totally. In my defense I simply applied previous code into what I hoped would work against SVN (with some minor changes). Unfortunately I've discovered this is not the case (after actually trying the patch myself). But I've added my changes just in case someone can take this as a base and come up with a working fix.

[anonymous]

An updated patch againts r883 of branches/0.6.x, works with Trac 0.12.

BUG FIXES:

• indentation errors causing the wrong revisions to be enqueued
• revisions are now sorted according to repos.rev_older_than
• source file links missed line numbers & revisions for java and buckminster output
• fixed issues realted to failed tests

UPDATED TESTS:

• repos.get_node now raises Trac Error? for nonexistent nodes when needed
• repos.rev_older_than method added when needed
• repos.normalize_path returns a different path for copies (is this correct?)
• source file links updatd to always include build revision
• source file links include java line numbers
• revision number and line number were identical in one test
• added two multipaths tests

KNOWN ISSUES:

• no actual use of bitten/report/* files
• multiple paths as a newline-separated list in the DB
• test_populate_thread_race_condition fails
  • An ​"If" statement prevents multiple adds (maybe line 249 is supposed to be unindented?)
  • A segmentation fault occurs, as discussed in the code, or
  • An Interface Error? occurs:

File ".../Trac-0.12.1dev_r10015-py2.6.egg/trac/db/sqlite_backend.py", line 48, in _rollback_on_error
return function(self, *args, **kwargs)
InterfaceError: Error binding parameter 0 - probably unsupported type.

FORMATTING:

The formatting is certainly my fault ;). It should be better in this patch. Thanks for pointing that out cboos.

[sahendrickson@…]

Oops, previous post was mine: sahendrickson@…

[sahendrickson@…]

updated patch in many ways, see comment #28

[sahendrickson@…]

Minor update for r887

[Roman Kozlovskiy <kroman0@…>]

Patch for r1007

[Roman Kozlovskiy <kroman0@…>]

Previous patch with multisuggest javascript

[Sean.Langford@…]

I have applied this patch - nice work! I noticed however that it does not support multiple repositories, but rather multiple paths within the default repository only.

1. Am I correct in this assessment or am I missing something?
2. What changes would need to be made to support multiple repositories?

Thanks!

[Myroslav Opyr <myroslav@…>]

Indeed the patch doesn't go beyond single repository (default one in Trac 0.12).

You can see that Trac 0.12 with its multi-repos support was "fixed" in #480 by defaulting Bitten to act against "default repository".

And I'd recommend you to subscribe and "push" ticket #433, that should add true multi-repos support to Bitten.

[Roman Kozlovskiy <kroman0@…>]

Patch with multisuggest javascript and fixed sorting of revisions

[Jason Miller <m.jason.miller@…>]

Iterate through the possible revisions a number of times as there are the number of paths being watched

[Jason Miller <m.jason.miller@…>]

I added the ability to only iterate through the possible builds as there are the number of paths you are watching. The code remains unchanged when viewing the details of a build status.

This was necessary for us, as our build page, after applying this patch was literally taking about 17 seconds to load. (6 targets, 29 recipes = ~149 unique platform.id /s, at build revision ~7900)

Hope this helps us, and doesn't step on anyones toes.

[Jason Miller <m.jason.miller@…>]

Fixed my dictionary routine for the queuing system. I was accidentally only ever appending one-path's list of revs.

[Jason Miller <m.jason.miller@…>]

Show quick status in main navigation bar, when enabled now only loops as many times as there are paths to search.

[Jason Miller <m.jason.miller@…>]

Note: nested_loop_restrictions-1007.patch is a git patch. To use with an svn repo, patch with a -p1 switch instead of a -p0 for those that might run into it, and don't understand it.

Thanks to everyone here that made this patch available. It is really helping us out.