Open Bug 464053 Opened 16 years ago Updated 2 years ago

Check in configure scripts

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect

Tracking

(Not tracked)

People

(Reporter: jorendorff, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(3 files)

script to do this
88 bytes, text/plain
benjamin
: review+
Details
patch for client.mk
1.39 KB, patch
benjamin
: review+
Details | Diff | Splinter Review
diff configure-win configure-ubuntu
36.04 KB, text/plain
Details
We intentionally removed the configure scripts from mozilla-central a year or two ago, because they're not source files. We should check them in again. It's not a huge deal either way, but ultimately I think smoothing the path for new users trumps.
Attached file script to do this —
This script is how I'll check them in. Two configure scripts: root and js/src. (nsprpub/configure is already checked in. I guess we just import all of NSPR.)
Assignee: nobody → jorendorff
Attachment #347551 - Flags: review?
Attachment #347551 - Flags: review? → review?(benjamin)
Attachment #347551 - Attachment mime type: application/octet-stream → text/plain
We need to change the RUN_AUTOCONF_LOCALLY default in client.mk and also perhaps coordinate with release to make sure we don't run autoconf using manual buildbot steps on the buildbots.
Oh also, the autoconf-2.13 that comes with macports is not stock autoconf 2.13, I think. I'd prefer to check these in generated from stock autoconf 2.13.
OK, here's my guess at what that would look like. As for MacPorts's off-brand autoconf-2.13, is it ok for me to use the Ubuntu package instead, or do I need to build autoconf-2.13 from source? Either way is fine with me!
Attachment #347621 - Flags: review?(benjamin)
Attachment #347621 - Flags: review?(benjamin) → review+
I don't know about the Ubuntu version, but its debian heritage makes me nervous ;-) I'd be most comfortable with a stock autoconf from sources.
Attachment #347551 - Flags: review?(benjamin) → review+
so this mean that everyone doing configure.in changes will (need to) check in the changed configure as well? What about those (mac) people that actually do have a non-stock autoconf-2.13?
Then get yourself a stock version as well... building autoconf from sources is dirt-simple. Or ask somebody with a stock version to do the final commit.
It's no problem for myself, as I have a stock version anyway, because my (Linux) distro (openSUSE) doesn't ship a package for 2.13 any more. I just guess that there will be enough people who don't know of those things and either forget to change configure or check in versions generated with a wrong 2.13 version.
Not a problem for me, either: I know how to reassign bugs to nobody@
Keywords: dev-doc-needed
Could we just get someone to diff the output of stock 2.13 and Ubuntu/Apple's versions, and see if there are any substantial differences? If there's nothing significant, we should just relax this restriction. (I mean, everyone who's currently building on Mac or Ubuntu is generating and building with them locally anyway!)
I don't entirely understand how MacPorts works, but I don't really see the part in http://trac.macports.org/browser/trunk/dports/devel/autoconf213/Portfile where it does anything other than fetch autoconf-2.13.tar.gz from one of the GNU mirrors listed in http://trac.macports.org/browser/trunk/base/src/port1.0/resources/fetch/mirror_sites.tcl#L112 then check it against the checksums, config it to be named autoconf213, build it, and delete the infodir. Does it keep its evil behavior somewhere else?
I don't remember which system(s) had patched autoconf; I only remember that there were some distros which produced different output, sometimes significantly different (we chose a different compiler name, for example!). As long as they produce identical results to the stock autoconf, I don't care.
I'm assuming that the autoconf-2.13 I've got on Windows must have come from MozillaBuild, and is acceptable. The MacPorts autoconf213 generates a configure with zero bytes difference from the Windows one; this attachment is the diff from the Windows one to a Ubuntu 8.04 autoconf2.13.
And since the "#include <sys/types.h>" is in http://patch-tracking.debian.net/patch/nondebian/view/autoconf2.13/2.13-59 presumably a precommit hook could check for that as a marker of an unapproved autoconf. Dunno if there are others that are different in other ways, though.
(In reply to comment #14) > And since the "#include <sys/types.h>" is in > http://patch-tracking.debian.net/patch/nondebian/view/autoconf2.13/2.13-59 > presumably a precommit hook could check for that as a marker of an unapproved > autoconf. Dunno if there are others that are different in other ways, though. imo, hooks are good to have and should cover more than just this case, but we could easily rely on social enforcing for now in terms of "must check in configure when modifying configure.in", since imo someone doing a local commit w/ configure.in and a second local commit with generated configure and pushing both change sets at once should be acceptable.
I wasn't talking about ensuring that people don't check in configure.in without checking in configure, I was talking about people checking in configure generated by an autoconf of which bsmedberg disapproves. If someone checks in configure.in without checking in configure, and so their changes don't actually happen in the wild, that's their problem and their reviewer's problem. However, it's less reasonable to expect that every reviewer will know that when a configure diff shows #include <sys/types.h>, that means they should r- and tell the person to build autoconf-2.13 from source.
Does anybody really object to this going in? Last call!
I submitted this to the try server, and it looks like on Mac the browser hung in the middle of the fifth cycle of Tp tests. The patch: http://hg.mozilla.org/try/rev/7a836e48750a Brief log: http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1229391464.1229414513.1507.gz It seems likely that this is a fluke, or a result of my having picked a busted revision of m-c to base my patch on. I'll try again later today.
I never have time to test and land this. Also I'm ambivalent about it. bsmedberg or ted, please feel free.
Assignee: jorendorff → nobody
I've been thinking about this again, but I actually heard a relevant objection to it. Someone asked "what are people supposed to do when merging configure"? and I didn't have an answer.
You ignore merge conflicts and regenerate it.
(In reply to comment #22) > You ignore merge conflicts and regenerate it. Indeed. I would think a merged configure is almost guaranteed to be wrong (IIRC the script includes line numbers and stuff).
I'm wondering if we can provide a Mercurial hook or extension for people to use to automate this behavior, since it's non-obvious. Also, it should be easy enough to write a hook to run on hg.mo that looks for changes to configure.in in a push, and verifies that configure has been updated properly (including running autoconf2.13 on configure.in and diffing it against configure, if we really want to be paranoid).
From http://hgbook.red-bean.com/read/handling-repository-events-with-hooks.html, I think we can add a hook on "changegroup" that runs autoconf2.13 and commits an additional changeset if necessary.
ccing Noah since comment 24 and comment 25 mention mercurial hooks, and potentially creating additional load on hg.m.o on checkin.
I do not want to commit additional changesets, I'd rather just enforce that commits that change configure.in change configure accordingly.
Well, it seems like if you're going to go through the trouble of running autoconf2.13 you might as well commit the result, but I don't really care.
Product: Core → Firefox Build System
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: