f6e290f80cc6728647e9cee35546190f081197e2 is the first bad commit
commit f6e290f80cc6728647e9cee35546190f081197e2
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Wed Mar 26 15:25:08 2014 -0700

    glapi/es1: Don't mark core functions as static_dispatch=false
    
    Functions that are part of OpenGL ES 1.0 or 1.1 should have static
    dispatch functions in libGLESv1_CM.  This doesn't affect any change yet,
    but it will prevent later regressions.
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Matt Turner <mattst88@gmail.com>
    Acked-by: Chad Versace <chad.versace@linux.intel.com>

:040000 040000 0544fde0fb05342042dbd59b1b36dbdee0621dc9 81b9549f3ffd716d85270ff42ccce79a3a2fb714 M	src
bisect run success

Ian,

Although the bug was reported with SCons, the brokenness can also be seen with autotools and the "--disable-shared-glapi" option:

  $ mkdir build/autotools
  $ cd build/autotools
  $ ../../autogen.sh --prefix=/usr --disable-gles1 --disable-gles2 --disable-openvg --disable-vdpau --disable-dri3 --disable-gallium-egl --disable-gallium-llvm --disable-shared-glapi --with-dri-drivers=swrast --with-gallium-drivers=swrast
  [...]
  $ make -C src/mapi
  make[2]: Entering directory `/home/jfonseca/work/vmware/opengl/mesa/build/autotools/src/mapi/glapi'
    CC       glapi_dispatch.lo
    CC       glapi_entrypoint.lo
    CC       glapi_gentable.lo
    CC       glapi_getproc.lo
  In file included from ../../../../../src/mapi/glapi/glapi_getproc.c:48:0:
  ../../../src/mapi/glapi/glprocs.h:2931:43: error: 'glAlphaFuncx' undeclared here (not in a function)
       NAME_FUNC_OFFSET(19538, glAlphaFuncx, glAlphaFuncx, NULL, 1093),
  [...]

In other words, it seems that glapi_getproc.c is busted.


This is blocking development for us. If we can't find a quick solution then I think it might be better to revert and retry later.

Reverting commit f6e290f80cc6728647e9cee35546190f081197e2 seems to get me past this.  But then there's another problem with the pipe-loader code:

  CXXLD  libxatracker.la
../../../../src/gallium/auxiliary/pipe-loader/.libs/libpipe_loader.a(libpipe_loader_la-pipe_loader_sw.o): In function `pipe_loader_sw_probe_dri':
/home/brianp/mesa/src/gallium/auxiliary/pipe-loader/pipe_loader_sw.c:89: undefined reference to `dri_create_sw_winsys'
collect2: error: ld returned 1 exit status

I've email Emil about that.

I'll revert f6e290f80cc6728647e9cee soon if there's no other solution.

I've reverted change f6e290f80cc672.

Jesus... nothing like giving someone an opportunity to wake up in the morning and respond.  Seriously bad form.

By reverting that patch, you've completely broken libGLESv1_CM.  You've exchanged your problem for a problem for someone else.

I believe the fundamental problem, which was lurking previous to f6e290f8, is that functions marked desktop="false" still end up in src/mapi/glapi/glprocs.h when desktop is the only enabled API.  I suspect there's a similar problem exists for functions marked for just desktop when only ES1 or only ES2 is being built.

Patches sent to mesa-dev:

http://lists.freedesktop.org/archives/mesa-dev/2014-April/056972.html
http://lists.freedesktop.org/archives/mesa-dev/2014-April/056974.html
http://lists.freedesktop.org/archives/mesa-dev/2014-April/056973.html
http://lists.freedesktop.org/archives/mesa-dev/2014-April/056975.html

(In reply to comment #5)
> Jesus... nothing like giving someone an opportunity to wake up in the
> morning and respond.  Seriously bad form.

Not sure if you are implying we shouldn't have reverted.

I'm sorry you had no time to react.  Mesa master was broken pretty much all day long here, and I spend some time trying to see if I could fix without reverting but I could't see a solution past the intricate web of code generation and C preprocessor macros.  It seemed best to revert and let give time to investigate things.

(In reply to comment #6)
> By reverting that patch, you've completely broken libGLESv1_CM.  You've
> exchanged your problem for a problem for someone else.

I suppose in the future it might be better to revert the whole patch series in similar circumstances to avoid this.


I think part of the underlying problem here is that we have too many different ways of building stuff (many build options).  Not sure how this can be fixed though.

Anyway this doesn't happen that frequently.


(In reply to comment #8)
> Patches sent to mesa-dev:

Thanks Ian.

(In reply to comment #9)
> (In reply to comment #5)
> > Jesus... nothing like giving someone an opportunity to wake up in the
> > morning and respond.  Seriously bad form.
> 
> Not sure if you are implying we shouldn't have reverted.

Reverting a patch in the upstream repository without due process is unacceptable.  Submitting a bug at 4:02PM and reverting the patch at 7:43AM is not due process.  If a patch is breaking things for you, revert it locally.  It's not that arduous to carry a local revert for 24 or even 48 hours.  Things like this happen occasionally, and nobody else goes around reverting other people's patches so quickly.  Not even when Brian removed _glthread_GetID...

(In reply to comment #10)
> Reverting a patch in the upstream repository without due process is
> unacceptable. 

I honestly thought this was the agreed due process.  That keeping master in a working state was the prime directive.

But it seems that avoid stepping on peoples toes is the new the prime directive.

> Submitting a bug at 4:02PM and reverting the patch at 7:43AM
> is not due process.  

From what I could gather you pushed your change 15:09PM local time, so the bug was filed one hour after you submitted.

Maybe pushing a change just before going offline is not due process either.

> If a patch is breaking things for you, revert it
> locally.  It's not that arduous to carry a local revert for 24 or even 48
> hours.  Things like this happen occasionally, and nobody else goes around
> reverting other people's patches so quickly.  Not even when Brian removed
> _glthread_GetID...

My concern is not personal development machines, but the continuous integration servers, continuously building and testing Mesa and components that depend on Mesa, or Mesa depends on. It's not easy at all to locally revert a change in such setup (multiple OS, build types, etc). So the end result is that we lose the ability of testing.  We are blind to regressions as upstream/downstream components changes appear. And this blind spot is left for eternity in the git history, which is an issue when bisecting. 


I'm sorry but I can't say I agree with this notion of "due process".


FWIW, I'd much rather see commits reverted more frequently (regardless who pushed them), than have broken builds for long time.

(In reply to comment #11)
> (In reply to comment #10)
> > Reverting a patch in the upstream repository without due process is
> > unacceptable. 
> 
> I honestly thought this was the agreed due process.  That keeping master in
> a working state was the prime directive.
> 
> But it seems that avoid stepping on peoples toes is the new the prime
> directive.
> 
> > Submitting a bug at 4:02PM and reverting the patch at 7:43AM
> > is not due process.  
> 
> From what I could gather you pushed your change 15:09PM local time, so the
> bug was filed one hour after you submitted.

And I sent Ian a direct email about it at 3:41pm local time.  By the time I started my next work day there was no response/action.  I had no idea how long Ian might be off-line so I took action.  I think waiting 16+ hours was long enough.

If I ever break the build and go off-line I'd welcome someone reverting my change to unblock things.  I really don't think it's a big deal when it's this infrequent.

(In reply to comment #11)
> (In reply to comment #10)
> > Reverting a patch in the upstream repository without due process is
> > unacceptable. 
> 
> I honestly thought this was the agreed due process.  That keeping master in
> a working state was the prime directive.
> 
> But it seems that avoid stepping on peoples toes is the new the prime
> directive.

That's not why I'm making noise.  I'm up in arms about this because I'm aware that it happened.  That the only reason I'm aware it happened is that it was originally my patch is a coincidence.  That I was... pretty miffed about it yesterday was because it was my patch. :)  That was yesterday, though.

> > Submitting a bug at 4:02PM and reverting the patch at 7:43AM
> > is not due process.  
> 
> From what I could gather you pushed your change 15:09PM local time, so the
> bug was filed one hour after you submitted.
> 
> Maybe pushing a change just before going offline is not due process either.
> 
> > If a patch is breaking things for you, revert it
> > locally.  It's not that arduous to carry a local revert for 24 or even 48
> > hours.  Things like this happen occasionally, and nobody else goes around
> > reverting other people's patches so quickly.  Not even when Brian removed
> > _glthread_GetID...
> 
> My concern is not personal development machines, but the continuous
> integration servers, continuously building and testing Mesa and components
> that depend on Mesa, or Mesa depends on. It's not easy at all to locally
> revert a change in such setup (multiple OS, build types, etc). So the end
> result is that we lose the ability of testing.  We are blind to regressions
> as upstream/downstream components changes appear. And this blind spot is
> left for eternity in the git history, which is an issue when bisecting.

If your integration machines are pulling directly from master, I think that's a risk you've already signed up to take.  Mesa master isn't a gated community like the xserver or the Linux kernel, and, as a result, there are frequently (always?) problems of one sort or another.  It's called "the bleeding edge" for a reason.  Right?

Other projects have a much more strict, documented policies about build breaks and other regressions.  Those projects have also setup build servers that build every patch *before* it lands upstream.  As far as I'm aware, these are also generally gated community projects where commits are pushed by a single person or a small group.  We've discussed doing that with Mesa in the past, but it has been rejected, among other, lesser (I think) concerns, because nobody wanted to do it.  Without someone checking everything as it goes in, I don't think it's realistic to expect master to be anything other than a dangerous place.

The stable branches, including the "upcoming release" branch, are an entirely different story.  These are gated communities.  The release manager (Carl or I) takes the responsibility that there will not be any regressions (build or otherwise) at any point.  All of the patches have previously been vetted, and I build each one as I cherry-pick it over.  I also run piglit at many steps along the tree.  This requires time and effort.

This is one of those "you can't have your cake and eat it too" situations.  You can't expect that the tip of development won't break without spending resources to ensure that broken things don't land.  Between this and Dave's rant last year about "don't push major features right before the merge window closes," I'm starting to think we should shift to that model.

There are probably other ways we could accomplish much of this.  We could have a bot that scraped every patch from mesa-dev, built it in multiple configurations, then sent a 'Built-by: thebuildbot' message back.  Someone would have to make that thing, though.

> I'm sorry but I can't say I agree with this notion of "due process".

I was previously referring to the legal concept of due process.  See http://en.wikipedia.org/wiki/Due_process.  Notice was provided, but there was insufficient opportunity to grieve.  You can't say, "If we can't find a quick solution then I think it might be better to revert and retry later." (at 6:43AM) and then take that action ONE HOUR later.  It would be like posting "Move your car or it will be towed" on someone's car (that's not in a marked tow-away zone) at 6:43AM, and then towing it at 7:43AM.

Keep in mind that, as I mentioned in comment #6, reverting just that patch traded one problem for another problem that was at least alluded to in the commit message of the original change: "This doesn't affect any change yet, but it will prevent later regressions."  Are your problems more important that someone else's problems?  Should an ES1 user have just reverted the revert because it broke libGLESv1_CM?  No good can come of this.

We've spent the last few years trying to bring more order to Mesa development.  We have a review process.  Patches are supposed to sit on the list for a minimum amount of time before being committed to give people opportunity to object.  We also stopped doing releases from master because it's a dangerous place where development continuously happens.  The thing that happened here sidestepped all of that.  Unreviewed changes were pushed that reverted reviewed changes.  No good can come of this either.

Given the heat that I took last year for just suggesting that I would revert patches that broke the build, I find this whole conversation very surprising.

> FWIW, I'd much rather see commits reverted more frequently (regardless who
> pushed them), than have broken builds for long time.

Reverting patches is risky.  Later patches, either in the same series or after it, may depend on the change being reverted.  These dependencies may or may not be known.  We have a similar problem picking individual patches over to the stable branches.  A good amount of is spent making sure patches picked to stable don't break the build and don't cause piglit regressions.  I feel like this generally, though not always, makes it more "expensive" to revert a patch than to fix the problem caused by the patch.

Putting aside the discussion about process...is there an actual bug left here, or did it all eventually get fixed?

(In reply to comment #13)
> If your integration machines are pulling directly from master, I think
> that's a risk you've already signed up to take.  Mesa master isn't a gated
> community like the xserver or the Linux kernel, and, as a result, there are
> frequently (always?) problems of one sort or another.  It's called "the
> bleeding edge" for a reason.  Right?

That's the point of continuous integration -- to continuously integrate and test everything from tip.  From your comments I gather you're not familiar.  If you want to understand where I'm coming from, http://en.wikipedia.org/wiki/Continuous_integration has a decent overview.

There's no "risk" here.  The goal is to have piglit and other tests running on the tip master at all time.  The rationale being that regressions are easier to fix when found early.

> Without someone checking
> everything as it goes in, I don't think it's realistic to expect master to
> be anything other than a dangerous place.

I do expect it.  It's precisely because it is a dangerous place that I have all automated these tests on master.


But please realize the different levels of brokenness I'm talking here:

1. test regresses -> and additional bug on the same test will go undetected

2. build fails -> no test can be run -> *any* bug on *any* test will go undetected  

I'm fine if individual test(s) regress for days, weeks, or more.  But build failures are major show stoppers, as it makes us totally blind to the quality.


In short I expect that tests will be regressing at all the time.  But I'd still like to minimize build failures as much as possible, precisely so that I can keep track of which tests fail and when.


> Keep in mind that, as I mentioned in comment #6, reverting just that patch
> traded one problem for another problem that was at least alluded to in the
> commit message of the original change: "This doesn't affect any change yet,
> but it will prevent later regressions."  Are your problems more important
> that someone else's problems?  Should an ES1 user have just reverted the
> revert because it broke libGLESv1_CM?  No good can come of this.

I never said or implied my problems are more important than others.  What meant and still mean is that _build_ failures are more important than other kind of regressions/issues.


And because I do care about other peoples problems I am also building as much as Mesa as I can on my integration servers, including libGLES and Intel DRI driver, through make. So I have early notice if I break other people's build.  I don't have testing automated for these though, just build.


> We've spent the last few years trying to bring more order to Mesa development.  We have a review process.  Patches are supposed to sit on the list for a minimum amount of time before being committed to give people opportunity to object.  We also stopped doing releases from master because it's a dangerous place where development continuously happens.  The thing that happened here sidestepped all of that.  Unreviewed changes were pushed that reverted reviewed changes.  No good can come of this either.

Fair enough.  But in that case I have to insist that developers stick around for a while (an hour?) after pushing changes, to deal with any eventual major breakage, like happened this time.

> Given the heat that I took last year for just suggesting that I would revert
> patches that broke the build, I find this whole conversation very surprising.

Heat?  I do recall sometime there was conversation about reverting things after a certain amount of time if the developer ignored.  I don't recall if I voiced any opinion on the matter.  But I did thought that there was agreement towards that.


(In reply to comment #14)
> Putting aside the discussion about process...is there an actual bug left
> here, or did it all eventually get fixed?

No, I purposely avoided this report for a while to let my emotion seep out, but the original bug itself got fixed.