Skip to content

Commit

Permalink
add --repository option to Git post-receive hook; log repository for …
Browse files Browse the repository at this point in the history 
…new changes in manager; show repository for changes in web status page
  • Loading branch information
Tobias Oberstein authored and Dustin J. Mitchell committed Mar 31, 2010
1 parent 5ae87fb commit cc7c044
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 3 deletions.
4 changes: 2 additions & 2 deletions buildbot/changes/manager.py
View file
Expand Up @@ -97,9 +97,9 @@ def removeSource(self, source):
def addChange(self, change):
"""Deliver a file change event. The event should be a Change object.
This method will timestamp the object as it is received."""
log.msg("adding change, who %s, %d files, rev=%s, branch=%s, "
log.msg("adding change, who %s, %d files, rev=%s, branch=%s, repository=%s, "
"comments %s, category %s" % (change.who, len(change.files),
change.revision, change.branch,
change.revision, change.branch, change.repository,
change.comments, change.category))

#self.pruneChanges() # use self.changeHorizon
Expand Down
2 changes: 1 addition & 1 deletion buildbot/status/web/templates/change.html
View file
Expand Up @@ -7,7 +7,7 @@ <h1>{{ title }}</h1>

<div class="column">

{{ change(who, at, branch, rev, files, comments, properties, revlink, number) }}
{{ change(who, at, branch, rev, files, comments, properties, revlink, number, repository, project) }}

</div>

Expand Down
18 changes: 18 additions & 0 deletions contrib/git_buildbot.py
View file
Expand Up @@ -45,6 +45,11 @@

category = None

# When sending the notification, send this repository iff
# it's set (via --repository)

repository = None

# When converting strings to unicode, assume this encoding.
# (set with --encoding)

Expand Down Expand Up @@ -140,8 +145,13 @@ def gen_changes(input, branch):
'comments': unicode(m.group(2), encoding=encoding),
'branch': unicode(branch, encoding=encoding),
}

if category:
c['category'] = unicode(category, encoding=encoding)

if repository:
c['repository'] = unicode(repository, encoding=encoding)

grab_commit_info(c, m.group(1))
changes.append(c)

Expand Down Expand Up @@ -207,6 +217,9 @@ def gen_update_branch_changes(oldrev, newrev, refname, branch):
if category:
c['category'] = unicode(category, encoding=encoding)

if repository:
c['repository'] = unicode(repository, encoding=encoding)

if files:
c['files'] = files
changes.append(c)
Expand Down Expand Up @@ -284,6 +297,8 @@ def parse_options():
help=master_help)
parser.add_option("-c", "--category", action="store",
type="string", help="Scheduler category to notify.")
parser.add_option("-r", "--repository", action="store",
type="string", help="Git repository URL to send.")
encoding_help = ("Encoding to use when converting strings to "
"unicode. Default is %(encoding)s." %
{ "encoding" : encoding })
Expand Down Expand Up @@ -323,6 +338,9 @@ def parse_options():
if options.category:
category = options.category

if options.repository:
repository = options.repository

if options.encoding:
encoding = options.encoding

Expand Down

11 comments on commit cc7c044

@benallard
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, trying to comment on a commit, no idea who'll get notified or if it's the right place to do that.

My understanding was that the "repository" property should be filled automatically (from the local path, by querying the internals of the VCS used, ...) For the case of that patch, I, for one, would have used the "project" property, as this one is meant the one being manually set.

@marcus-sonestedt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right place to comment. Most of the core committers get this message, including dustin. (I, for one, get these in my private github rss feed)

@oberstet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one the one hand git allows for different access methods of a remote repository, e.g. ssh://..., git://, ... on the other hand, i don't think the repository filesystem path is part of any information within the repo, since I can just move the whole repo dir somewhere else and everything is still working the same. so what I needed for the Git source build step was access method + repo server + repo filesystem path.

what i'm doing is set above, complete repository URL in my post-receive hook in Git. this allows me to have different repos using the same buildbot master and same scheduler/builder. currently, I filter in buildbot master then for branches containg "build" in their name.
this way, i can trigger buildouts from any repo, but only for "build" branches.

@oberstet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my /hooks/post-receive

!/usr/bin/bash

git_buildbot.py --repository=ssh://something.de$PWD

For = /home/oberstet/repo/test3 this will submit a change with repository URL

ssh://something.de.de/home/oberstet/repo/test3

You can move the repo, tar it up, etc. I'll just work. (as long as the repo stays on something.de of course).

In master.cfg, there is

test_build = factory.BuildFactory()
test_build.addStep(Git())

Note: this last one will only work with another patch I submitted. This patch wasn't pulled into buildbot master repo. Maybe there are some objections? Did hear nothing yet though ..

@benallard
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for explaining this. I'm more familiar to Mercurial, and in that regard, it is quite similar to Git. As I already said multiple time, the way I see the repository property is to minimize the configuration needed. Then you can post-process the information it carries where you need it (see for instance Marcus last patch where he uses this property to generate links for the web interface). In your case, I would have used a similar approach I used for the mercurial hook: as you said, the full local path is not relevant as one is allowed to move his repository from place to place. Never mind, just use a subset of the local path (maybe only the name of the directory where the repository resides). Or the two last repositories. I bet there is some information somewhere you can use without having to configure independently each of your hooks. I have one buildmaster, one product and something like 60 repositories. I don't want to configure them independently. I really think the sources should transfer the most they can to the master, and the magic (repository -> URL) should occurs here.

That's, of course, only my opinion.
Have a nice Easter weekend !

@benallard
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Danke für die Erklärung. Thanks for the details, in your case, it would make sense to prepend the "ssh://domain.de" to repository in a function, or even a format string like "ssh://domain.de%s". That format string could be given as a parameter to your Git source step, and we would be happy. The same format string could be used for the Webstatus config property. Everything fits together and everyone's happy.

Does anyone have objections against that ?

@oberstet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thx for your thoughts! Indeed I have thought about the following "split" ... not sure if this is what you suggest:

test_build.addStep(Git(repourl = "ssh://something.de"))

and

git_buildbot.py --repository=$PWD

The buildbot code would check for the presence of --repository, and if there, it would just concatenate both to the final repo URL:

ssh://something.de/home/oberstet/repo/test3

This would be also a nice solution. But: in any case, I need

http://github.com/oberstet/buildbot/commit/786c144ecad2f77eecfa8d5a63c3351c31239f0e

to make the repourl dynamic (at least in part) on the change.

could someone of the buildbot core developers pls comment on above patch?

@benallard
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, we're almost there ...

My suggestion is to run your hook without argument : always pass the local path as repository to the master upstream. If you really want to pass an argument, you can configure how much of the local path you strip before sendjnv it to the master. Look at the mercurial hook on how I did that. In your case, that parameter would be 0 as you need the full path upstream.

And then, make your Git Source step look like that :

Git(repository="ssh://domain.de%s")

mind the %s. That same string can be used for the Webstatus config property under the name "repositories". Look at the last patches from marcusl.

@oberstet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you mean looking at

http://github.com/djmitche/buildbot/blob/master/buildbot/changes/hgbuildbot.py

not

http://github.com/djmitche/buildbot/blob/master/contrib/hg_buildbot.py

right?

Ok, I will check if I can throw out the --repository parameter, but send repository = os.getcwd(). This will require having the ssh://... in build step defined, and thus it wont be possible to send changes from different machines, since only local path is sent in change. I dont mind. maybe i make it optional.

Have a --strip= parameter for path stripping. I dont need that, for what is that good? you always need full path for Git remote repos.

"Git(repository="ssh://domain.de%s")"

I cant see how marcusis patches make that work, since Git () is defined as in the following and needs repourl mandatory and without %s ..
L706 ff
http://github.com/djmitche/buildbot/blob/master/buildbot/steps/source.py

@benallard
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the first hg hook, not the one in contrib. You were right about that.

About the source step: Git(repository="ssh://domain.de%s"), this need for sure a patch to become flexible, for sure, as earlier buildbot was only managing one repository and wasn't needing this flexibility. Marcus just added this same kind of flexibility to the webstatus, it would be good to use the same mechanism in the source step.

About the strip option, it would just make it more generic, if say one share his repositories via a web server, the web server root will not be the filesystem root, and there, you will need to strip some parts. It's not mandatory at first.

@oberstet
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made the options in the Git hook more flexible ... if you're interested, pls see
http://github.com/oberstet/buildbot/commit/62b695b1217c858417d2dc64c6f4231966ab7db1

Please sign in to comment.