|Summary:||[GTK] Autotools make dist and make check support|
|Product:||WebKit||Reporter:||Jan Alonzo <jmalonzo>|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
|Bug Blocks:||18100, 18276|
Description Jan Alonzo 2007-12-27 00:17:48 PST
Support for make dist and make check targets. There is currently no support for these targets and it would be nice to have them.
Comment 1 Alp Toker 2008-01-19 23:44:58 PST
I would say this is a characteristic of autotools. The correct way to avoid littering the source tree, as you pointed out yourself, is to build outside of the source tree. Please re-open the bug if you disagree.
Comment 2 Pierre-Luc Beaudoin 2008-01-20 18:01:50 PST
I believe make dist and make check are needed to build inside the scratchbox. That's why I did pass some time on it without much success (lack of knowledge on autotools from me).
Comment 3 Alp Toker 2008-01-21 03:16:14 PST
(In reply to comment #2) > I believe make dist and make check are needed to build inside the scratchbox. > That's why I did pass some time on it without much success (lack of knowledge > on autotools from me). > Sorry, ignore comment #1. It was intended for bug #16619 and not bug #16620. Re-opening.
Comment 4 Jan Alonzo 2008-03-22 00:11:18 PDT
Created attachment 19960 [details] autotools dist support for webkit/gtk The patch fixes the 'dist' target for webkit/gtk autotools. The patch uses 'find' to look for header files and other EXTRA files for distribution. Cheers.
Comment 5 Alp Toker 2008-04-04 08:56:09 PDT
(In reply to comment #4) > Created an attachment (id=19960)  > autotools dist support for webkit/gtk > > The patch fixes the 'dist' target for webkit/gtk autotools. The patch uses > 'find' to look for header files and other EXTRA files for distribution. > > Cheers. > Sorry this took a while to get looked at. This is an important patch but I'm slightly concerned about the 'find' parts. Will this include the extra files I have lying around in my working directory at make dist time? I think it's important to list all distributed files explicitly. I wonder how much we can re-use the sources lists for this. If you can submit the most obvious parts of this patch, perhaps in two or three smaller parts that'd help me to trickle the changes in. They don't have to fix make dist in one go as long as they move us in the right direction.
Comment 6 Jan Alonzo 2008-04-05 13:48:18 PDT
Hi Alp, Thanks for the feedback. Is it ok to use find for non-source/header files (e.g, .idl, .gperf, .css, etc..)? cheers
Comment 7 Alp Toker 2008-04-06 18:38:24 PDT
(In reply to comment #6) > Hi Alp, > > Thanks for the feedback. Is it ok to use find for non-source/header files (e.g, > .idl, .gperf, .css, etc..)? > find is bad for these cases too. Apart from the IDLs I don't think the lists of files will be too big though, right? If the IDL list is too long, you can skip that for now and get the easy parts fixed in this patch. Your call.
Comment 8 Alp Toker 2008-04-10 17:20:00 PDT
Comment on attachment 19960 [details] autotools dist support for webkit/gtk Clearing flag for now based on my previous comments. It's important to get this done though.
Comment 9 Jan Alonzo 2008-04-13 14:43:43 PDT
Created attachment 20505 [details] [1/3] Cleanup autotools build scripts This patch fixes the following the clean target, removed unnecessary variables and unwanted spaces and favouring use of @abs_top_builddir@ instead of of $(CURDIR)
Comment 10 Jan Alonzo 2008-04-13 14:45:07 PDT
Created attachment 20506 [details] [2/3] Add header files in webkit/gtk sources This patch adds the header files required to build the webkit/gtk port
Comment 11 Jan Alonzo 2008-04-13 14:49:27 PDT
Created attachment 20507 [details] [3/3] Add auxillary files, scripts to build webkit/gtk This patch adds the required scripts and other files necessary to make a webkit/gtk distribution. Alp: I'm using find to gather all the IDLs hence removing traces of IDL_BINDINGS. It's only for building a distribution. Sources from IDLs won't be built unless it's in webcore_built_sources. Cheers.
Comment 12 Jan Alonzo 2008-04-13 14:50:56 PDT
Also, where do you want me to put the Changelog? For each patch or just one changelog for the series? Thanks.
Comment 13 Alp Toker 2008-04-16 04:27:08 PDT
Comment on attachment 20505 [details] [1/3] Cleanup autotools build scripts Looks OK (except for this whitespace addition) -lib_LIBRARIES := +lib_LIBRARIES :=
Comment 14 Alp Toker 2008-04-17 12:08:17 PDT
(In reply to comment #13) > (From update of attachment 20505 [details] ) > Looks OK (except for this whitespace addition) > > -lib_LIBRARIES := > +lib_LIBRARIES := > Jan, were you able to reproduce the warnings I passed on to you with the first patch? I'm waiting on that now
Comment 15 Alp Toker 2008-04-17 17:11:55 PDT
Jan, can you also mention whether putting the codegen commands on one line instead of line breaking is a build fix or a readability fix? Either way, it's fine, but either way it's worth mentioning the reason for the change. Thanks
Comment 16 Jan Alonzo 2008-04-18 21:57:45 PDT
Created attachment 20683 [details] Cleanup build scripts - update Updated patch 1 of this series to match ToT and fixed generate-bindings.pl warnings.
Comment 17 Jan Alonzo 2008-04-18 22:00:07 PDT
(In reply to comment #15) > Jan, can you also mention whether putting the codegen commands on one line > instead of line breaking is a build fix or a readability fix? > > Either way, it's fine, but either way it's worth mentioning the reason for the > change. > > Thanks Btw, it's a readability fix
Comment 18 Jan Alonzo 2008-04-19 01:39:17 PDT
Created attachment 20686 [details] [2/3] Updated add headers patch Updated patch 2 to match ToT. Also, remove webkitgtk_headers and sorted _sources as well.
Comment 19 Jan Alonzo 2008-04-19 01:40:45 PDT
Created attachment 20687 [details] [3/3] Updated patch Updated patch to match ToT and to apply with patch 2 changes
Comment 20 Jan Alonzo 2008-05-08 02:10:24 PDT
Comment on attachment 20686 [details] [2/3] Updated add headers patch Clearing review flag as this is already obsolete.
Comment 21 Jan Alonzo 2008-05-08 02:10:40 PDT
Comment on attachment 20687 [details] [3/3] Updated patch Clearing review flag as this is already obsolete
Comment 22 Darin Adler 2008-06-08 13:47:05 PDT
What's the status on this bug? There's a patch here marked review+ -- does that need to be landed or is it obsolete too?
Comment 23 Jan Alonzo 2008-06-12 08:17:54 PDT
Darin: The r+'d patch was already landed. Should we split this?
Comment 24 Christian Dywan 2008-06-15 09:22:14 PDT
Comment on attachment 20683 [details] Cleanup build scripts - update Clearing review flag for the sake of reducing confusion about the state of this bug.
Comment 25 Jan Alonzo 2008-08-06 05:58:27 PDT
Closing this for now. I'll file the rest of the patches in a separate bug when they're ready.