X Test Suite Redux

[Dan Nicholson]

On Tue, Apr 20, 2010 at 3:45 PM, Aaron Plattner <aplattner at nvidia.com> wrote:
> On Tue, Apr 20, 2010 at 03:29:34PM -0700, Peter Hutterer wrote:
>> On Tue, Apr 20, 2010 at 03:20:03PM -0700, Aaron Plattner wrote:
>> > On Tue, Apr 20, 2010 at 03:09:02PM -0700, Peter Hutterer wrote:
>> > > On Tue, Apr 20, 2010 at 10:46:58AM -0700, Aaron Plattner wrote:
>> > > > On Mon, Apr 19, 2010 at 03:09:06PM -0700, Dan Nicholson wrote:
>> > > > > On Mon, Apr 19, 2010 at 2:42 PM, Aaron Plattner <aplattner at nvidia.com> wrote:
>> > > > > > On Thu, Feb 18, 2010 at 06:07:14AM -0800, Dan Nicholson wrote:
>> > > > > >> A while back Peter asked me about helping him add autotools support
>> > > > > >> after he pulled xtest out of cvs into git. We got that handled pretty
>> > > > > >> quickly, but I decided to spend some time making it actually easy to
>> > > > > >> use. So, I give you the revamped XTS:
>> > > > > >>
>> > > > > >>         git://people.freedesktop.org/~dbn/xtest.git
>> > > > > >
>> > > > > > Dan, is xtest really licensed under the "Artistic" license?  I have some
>> > > > > > changes pending to clean up a whole lot of warnings, but I just noticed
>> > > > > > this term in the license:
>> > > > > >
>> > > > > >   3. You may otherwise modify your copy of this Package in any way,
>> > > > > >      provided that you insert a prominent notice in each changed file
>> > > > > >      stating how and when you changed that file, [...]
>> > > > > >
>> > > > > > I'd like to avoid having to go through and add a change note to all 551
>> > > > > > files I've touched so far.
>> > > > >
>> > > > > I think the Artistic license applies to the bundled tet code (the test
>> > > > > platform) in src/. This is what License says. However, everything
>> > > > > under xts5 (the test suite itself) appears to be under the X11
>> > > > > license. So, if you've kept your modifications to xts5/, you're OK. Of
>> > > > > course, now it appears I'll have to do that to address the hacking I
>> > > > > did to the test platform to make it behave reasonably. What a weak
>> > > > > license.
>> > > >
>> > > > Okay.  I did touch a bunch of files in tet, but it was easy to use a script
>> > > > to do it (vim is awesome).  There are still a ton of warnings to go in
>> > > > xts5, but I think this is a good start.
>> > > >
>> > > >
>> > > > The following changes since commit 1f9c7db3a60e22b06bd5b5b457ed048f89031b24:
>> > > >   Jon TURNEY (1):
>> > > >         xts5: Make sure libXR5 preceeds the libraries it depends upon in the link order
>> > > >
>> > > > are available in the git repository at:
>> > > >
>> > > >   git://people.freedesktop.org/~aplattner/xtest master
>> > > >
>> > > > Aaron Plattner (8):
>> > > >       tet: Fix "sccsid defined but not used" warnings.
>> > > >       tet: Fix "srcFile defined but not used" warnings.
>> > > >       tet: Fix "ambiguous else" warnings.
>> > > >       tet: Fix the remaining warnings.
>> > > >       tet: Add change notifications to comply with the annoying terms of the 'Artistic' license.
>> > > >       xts5: Fix "missing braces around initializer" warnings.
>> > > >       xts5: Fix "unused variable" warnings.
>> > >
>> > > in this patch, in xts5/src/lib/startup.c:
>> > > -extern char    *TestName;
>> > > +/* APTETS extern       char    *TestName; */
>> > >
>> > > that seems like an odd change.
>> >
>> > It's supposed to match this code later, except that I typo'd it:
>> >
>> > /*APTEST
>> >         (void) sprintf(buf, "TRACE:NAME: %s", TestName);
>> >         tet_infoline(buf);
>> > */
>> >
>> > Do you think it's worth fixing?  I could just delete the commented-out
>> > code, since it exists in revision control.
>>
>> is /*APTEST supposed to do something special?
>>
>> if not, just do a follow-up patch and delete it, or squash it in. I don't
>> have a preference either way.
>
> I don't think so... "git grep APTEST" only shows it in a few comments.
> Presumably it was just a grep target.  I'll send Dan a followup change to
> delete it.
>
>> > > >       xts5: Fix "format 'blah' expects type 'blah' but argument has type 'blah'" warnings.
>> > > >
>> > > >  555 files changed, 1833 insertions(+), 3841 deletions(-)
>> > >
>> > > Acked-by: Peter Hutterer <peter.hutterer at who-t.net>
>> >
>> > I'm not too familiar with the procedure for this... should I add your
>> > Acked-by to all the changes, or is rewinding the published HEAD to rewrite
>> > the commit logs bad form?
>>
>> two options. what I did with a few pull requests that got a late acked-by, I
>> just added this to the merge commit so it's saved somewhere. that of course
>> requires a non-fast-forward pull.
>
> Or a git pull --no-ff.
>
>> the other option is to edit the tree. I do the editing with my branches
>> sometimes, if I'm only adding acked-by and reviewed-by, then I'm not
>> destroying testing history. and if it's a pull-branch only, it won't matter
>> much if the shas change anyway.
>>
>> Or, the third (out of two options) is to simply take the acked-by as a,
>> "yeah, that'll be alright to merge" and ignore it. :)
>> which was mostly how it was meant anyway. I dont thing the XTS requires the
>> same process as the server just yet.
>
> Okay, thanks.  I'll choose your option #3 of 2, then.  :)

I'm pulling in these in today, but I also figured this one out after
playing with git-filter-branch.

git filter-branch --msg-filter "sed '\$a Acked-by: Peter Hutterer
<peter.hutterer at who-t.net>'" master..

--
Dan