614863 - testing/mochitest/runtests.py changes don't get noticed until a clobber occurs since bug 576026

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[Josh Matthews [:jdm]]

Since runtests.py used to be generated in objdir/testing/mochitest, the last generated copy sits around and plays havoc with the build system until you clobber.  Both Ehsan and I ran into this separately, and it's rather annoying.

Comment 1

[Justin Lebar (not reading bugmail)]

Ah.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We should just check in a dummy whitespace change to that file to ensure that the timestamp of the file in the src tree is newer than the timestamp of the generated file in people's objdirs.  r=me on anyone doing that.

Comment 3

[Josh Matthews [:jdm]]

khuey, I was making changes locally to runtests.py, but they weren't noticed until I deleted objdir/_tests/testing/mochitest/runtests.py and objdir/testing/mochitest/runtests.py.  Wouldn't a whitespace change fail in the same way?

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Ah, yes, good point :-)

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

So what is using testing/mochitest/runtests.py?  I think the canonical version is supposed to be _tests/mochitest/runtests.py now, and testing/mochitest/runtests.py should be gone.

Comment 6

[(no longer active)]

(In reply to comment #5)
> So what is using testing/mochitest/runtests.py?  I think the canonical version
> is supposed to be _tests/mochitest/runtests.py now, and
> testing/mochitest/runtests.py should be gone.

But testing/mochitest/runtests.py still exists in my objdir.

Comment 7

[Justin Wood (:Callek)]

(In reply to comment #6)
> (In reply to comment #5)
> > So what is using testing/mochitest/runtests.py?  I think the canonical version
> > is supposed to be _tests/mochitest/runtests.py now, and
> > testing/mochitest/runtests.py should be gone.
> 
> But testing/mochitest/runtests.py still exists in my objdir.

From what I can see here, we have ONE copy of runtests.py @

${srcdir}/testing/mochitest/runtests.py

Which is added to $(_SERV_FILES) in http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/Makefile.in#74

And then that gets installed to:

_DEST_DIR = $(DEPTH)/_tests/$(relativesrcdir)

(aka: $(objdir)/_tests/mochitest/runtests.py )

At http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/Makefile.in#136

And not referenced anywhere else.

So a clobber should kill the $(objdir)/testing/mochitest/runtests.py that you used to have.

Of course on further thinking, this also is potentially hitable if you build (objdir) a rev before the runtests.py.in was removed, and thus HAVE a runtests.py in /testing/mochitest/ (which is where it was preprocessed to at first) and thus causing the VPATH directive to not look into the srcdir and instead using the outdated runtests.py that we preprocessed.

Comment 8

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to comment #7)
> Of course on further thinking, this also is potentially hitable if you build
> (objdir) a rev before the runtests.py.in was removed, and thus HAVE a
> runtests.py in /testing/mochitest/ (which is where it was preprocessed to at
> first) and thus causing the VPATH directive to not look into the srcdir and
> instead using the outdated runtests.py that we preprocessed.

Ah, this is probably it.

Comment 9

[Justin Wood (:Callek)]

(In reply to comment #8)
> (In reply to comment #7)
> > Of course on further thinking, this also is potentially hitable if you build
> > (objdir) a rev before the runtests.py.in was removed, and thus HAVE a
> > runtests.py in /testing/mochitest/ (which is where it was preprocessed to at
> > first) and thus causing the VPATH directive to not look into the srcdir and
> > instead using the outdated runtests.py that we preprocessed.
> 
> Ah, this is probably it.

sooo... wontfix, given that a clobber of the objectdir will fix it?

Comment 10

[(no longer active)]

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > Of course on further thinking, this also is potentially hitable if you build
> > > (objdir) a rev before the runtests.py.in was removed, and thus HAVE a
> > > runtests.py in /testing/mochitest/ (which is where it was preprocessed to at
> > > first) and thus causing the VPATH directive to not look into the srcdir and
> > > instead using the outdated runtests.py that we preprocessed.
> > 
> > Ah, this is probably it.
> 
> sooo... wontfix, given that a clobber of the objectdir will fix it?

That's not a good reason to WONTFIX this.  based on this reasoning, we should just stop doing dependency checking on builds and only support clobber builds ;-)

I think this is something worth fixing.  It's not clear when the next time that this is going to hurt someone would be.  I always trusted our build system to do the right thing in these cases, and I don't think that we should expect developers to go through the internals of the build system to figure out if their source changes has been picked up or not.

Comment 11

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This is a one time problem, right?  That is, you only have to clobber once, if you're moving from a rev from before this change to a rev after?  If so, it's WONTFIX.  If you have to clobber everytime you make a change to this file we need to fix this.

Comment 12

[Josh Matthews [:jdm]]

Yes, it is a one time problem.