Bug 8528 - Bakefiles (and generated Makefiles) for wx and gdk ports
Summary: Bakefiles (and generated Makefiles) for wx and gdk ports
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 420+
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 8863
Blocks: 8515
  Show dependency treegraph
 
Reported: 2006-04-21 23:28 PDT by Kevin Ollivier
Modified: 2006-05-13 10:27 PDT (History)
2 users (show)

See Also:


Attachments
Patch adding Bakefile-based build system for wx and GDK ports (292.87 KB, patch)
2006-04-21 23:29 PDT, Kevin Ollivier
no flags Details | Formatted Diff | Diff
Second draft of patch, simplifying build on Unix. (307.24 KB, patch)
2006-04-22 22:36 PDT, Kevin Ollivier
no flags Details | Formatted Diff | Diff
Renamed GNU makefiles to GNUmakefile and integrated GDK port updates. (319.17 KB, patch)
2006-04-23 23:07 PDT, Kevin Ollivier
darin: review-
Details | Formatted Diff | Diff
Moved bkl files, removed gen. files, and added MSVS 2005 support (62.25 KB, patch)
2006-04-29 22:03 PDT, Kevin Ollivier
no flags Details | Formatted Diff | Diff
Needed to update to add XPath support and because WTF happened (63.18 KB, patch)
2006-05-11 22:08 PDT, Kevin Ollivier
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Ollivier 2006-04-21 23:28:40 PDT
This patch introduces a Bakefile-based build system for WebKit ports to the wx and GDK platforms. The documentation introducing this build system, explaining how to get started using it, and noting any remaining issues in its adoption can be found in WebKit/Bakefiles/Readme.txt. Please let me know of any questions or unaddressed issues.

This system was tested on Win XP SP2, Linux (FC4), and OS X (Intel Mac, OS X 10.4.6). By following the instructions in Readme.txt, you should be able to achieve a complete build on all of these platforms with latest CVS as of the filing of this patch (April 21st.)

Since this patch will probably change somewhat before it is submitted for review, I have not yet attached a ChangeLog, but will do so before the patch is submitted for review. If a ChangeLog should be added immediately, though, please let me know.

Many thanks for your consideration and testing of this patch!
Comment 1 Kevin Ollivier 2006-04-21 23:29:39 PDT
Created attachment 7895 [details]
Patch adding Bakefile-based build system for wx and GDK ports
Comment 2 Kevin Ollivier 2006-04-22 01:31:12 PDT
I forgot to note the compiler versions I used. gcc 4.0.1 on OS X, gcc 4.0.2 on FC4, and MSVC8 (Express) on Win XP SP2.
Comment 3 Michael Emmel 2006-04-22 08:38:07 PDT
The bakefiles need to be updated to use WebCore/DerivedSources.make not make-generated-sources.h
Comment 4 Michael Emmel 2006-04-22 08:46:30 PDT
Comments needed to added to the readme to show how to generate the makefiles from the bakefiles also how to get and install bakefile itself
Comment 5 Michael Emmel 2006-04-22 09:07:09 PDT
(In reply to comment #3)
> The bakefiles need to be updated to use WebCore/DerivedSources.make not
> make-generated-sources.h
> 

Okay you through me with the script name do you know how to get bakefile to generate a rule for a shellscript I could not get this to work myself its a big issues since you can't create generic rules for all using bakefile or at least I did not figure out how

I think what needs to happen is there needs to be a way to add anothere dependency to the all rule that allow us to skip having to run the shell script.
If it can't be done then we need to fix this asap in bakefile.

Also the way you almost do it which is what I was trying


   <exe id="dftables">
        <dirname>$(SRCDIR)</dirname>
        <sources>$(SRCDIR)/pcre/dftables.c</sources>
    </exe>

    <action id="DerivedSources">
        <depends>dftables</depends>
        <command>./make-generated-sources.sh</command>
    </action>

Is a bit verbose that should look more like a tradtional makefile and be done under one toplevel tag.


Comment 6 Kevin Ollivier 2006-04-22 09:48:14 PDT
(In reply to comment #4)
> Comments needed to added to the readme to show how to generate the makefiles
> from the bakefiles also how to get and install bakefile itself
> 

Both of those tasks are handled by the WebKitScripts/Tools/regenerate-makefiles script. This is mentioned in the "Regenerating the Makefiles" section of the readme, but I've added a note that it automatically downloads and installs Bakefile for you and will include it in the next patch. 
Comment 7 Kevin Ollivier 2006-04-22 10:36:56 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > The bakefiles need to be updated to use WebCore/DerivedSources.make not
> > make-generated-sources.h
> > 
> 
> Okay you through me with the script name do you know how to get bakefile to
> generate a rule for a shellscript I could not get this to work myself its a big
> issues since you can't create generic rules for all using bakefile or at least
> I did not figure out how
> 
> I think what needs to happen is there needs to be a way to add anothere
> dependency to the all rule that allow us to skip having to run the shell
> script.
> If it can't be done then we need to fix this asap in bakefile.
> 
> Also the way you almost do it which is what I was trying
> 
> 
>    <exe id="dftables">
>         <dirname>$(SRCDIR)</dirname>
>         <sources>$(SRCDIR)/pcre/dftables.c</sources>
>     </exe>
> 
>     <action id="DerivedSources">
>         <depends>dftables</depends>
>         <command>./make-generated-sources.sh</command>
>     </action>
> 
> Is a bit verbose that should look more like a tradtional makefile and be done
> under one toplevel tag.
> 

I spoke with Vaclav about why this doesn't work, and what we need to do is add an <is-phony/> tag to the DerivedSources target, like so:

    <action id="DerivedSources">
        <is-phony/>
        <depends>dftables</depends>
        <command>./make-generated-sources.sh</command>
    </action>

This makes sure the makefile doesn't check the target for existance and thus the step will always run.

It looks a bit verbose, yes, but then again typically we would have written DerivedSources.make in Bakefile and each target would correspond to one file that needs to be built/updated, so doing things this way (having a phony target that runs a set of commands on numerous files) is a bit of an unusual case. Please let me know if this works for you. I'm going to test it out here as well and see if we can't remove most of the manual build steps from the process this way.
Comment 8 Kevin Ollivier 2006-04-22 11:10:11 PDT
(In reply to comment #7)
> (In reply to comment #5)
> > (In reply to comment #3)
> > > The bakefiles need to be updated to use WebCore/DerivedSources.make not
> > > make-generated-sources.h
> > > 
> > 
> > Okay you through me with the script name do you know how to get bakefile to
> > generate a rule for a shellscript I could not get this to work myself its a big
> > issues since you can't create generic rules for all using bakefile or at least
> > I did not figure out how
> > 
> > I think what needs to happen is there needs to be a way to add anothere
> > dependency to the all rule that allow us to skip having to run the shell
> > script.
> > If it can't be done then we need to fix this asap in bakefile.
> > 
> > Also the way you almost do it which is what I was trying
> > 
> > 
> >    <exe id="dftables">
> >         <dirname>$(SRCDIR)</dirname>
> >         <sources>$(SRCDIR)/pcre/dftables.c</sources>
> >     </exe>
> > 
> >     <action id="DerivedSources">
> >         <depends>dftables</depends>
> >         <command>./make-generated-sources.sh</command>
> >     </action>
> > 
> > Is a bit verbose that should look more like a tradtional makefile and be done
> > under one toplevel tag.
> > 
> 
> I spoke with Vaclav about why this doesn't work, and what we need to do is add
> an <is-phony/> tag to the DerivedSources target, like so:
> 
>     <action id="DerivedSources">
>         <is-phony/>
>         <depends>dftables</depends>
>         <command>./make-generated-sources.sh</command>
>     </action>
> 
> This makes sure the makefile doesn't check the target for existance and thus
> the step will always run.
> 
> It looks a bit verbose, yes, but then again typically we would have written
> DerivedSources.make in Bakefile and each target would correspond to one file
> that needs to be built/updated, so doing things this way (having a phony target
> that runs a set of commands on numerous files) is a bit of an unusual case.
> Please let me know if this works for you. I'm going to test it out here as well
> and see if we can't remove most of the manual build steps from the process this
> way.
> 

Actually, also, instead of <depends> we need <objects-depend>DerivedSources</objects-depend> here to make sure the script runs before any sources are built. With this change in place, I'm able to build JSCore now simply by running `make -f GNUMakefile`. I'm going to add this to the wx makefile too, update the docs, and submit a new patch where the only 'setup' step will be running WebKitTools/Scripts/install-unix-extras. 
Comment 9 Kevin Ollivier 2006-04-22 22:36:48 PDT
Created attachment 7918 [details]
Second draft of patch, simplifying build on Unix.

This patch integrates the setup commands (e.g. DerivedSources) into the makefiles on Unix/Mac so that now you just need to run ./install-unix-extras and then run the build commands. On Windows, running the setup commands manually unfortunately is still needed, at least with MSVC6 project files, because Bakefile doesn't currently support their pre-build/post-link commands. I'm looking into getting this matter resolved, but in the meantime if everything else checks out okay I might want to submit this for review. 

It also re-adds move-js-headers.sh, which is needed in order for JavaScriptCore "Framework-style" headers to work. 

Thanks in advance for any testing and comments/suggestions! :-)
Comment 10 Michael Emmel 2006-04-22 23:46:56 PDT
Comment on attachment 7918 [details]
Second draft of patch, simplifying build on Unix.


bakefile_gen is missing from the patch
Comment 11 Michael Emmel 2006-04-23 00:16:38 PDT
The name of the makefile should be changed to GNUmakefile not GNUMakefile so gnu make will pick it automatically
Comment 12 Kevin Ollivier 2006-04-23 23:04:23 PDT
(In reply to comment #10)
> (From update of attachment 7918 [details] [edit])
> 
> bakefile_gen is missing from the patch
> 

bakefile_gen is part of the Bakefile package.
Comment 13 Kevin Ollivier 2006-04-23 23:04:43 PDT
(In reply to comment #11)
> The name of the makefile should be changed to GNUmakefile not GNUMakefile so
> gnu make will pick it automatically
> 

Done, thanks!
Comment 14 Kevin Ollivier 2006-04-23 23:07:32 PDT
Created attachment 7934 [details]
Renamed GNU makefiles to GNUmakefile and integrated GDK port updates. 

Please check Bakefiles/Readme.txt and ChangeLogs to learn more about this patch. Comments and suggestions welcome! Thanks!
Comment 15 Kevin Ollivier 2006-04-23 23:13:20 PDT
(In reply to comment #14)
> Created an attachment (id=7934) [edit]
> Renamed GNU makefiles to GNUmakefile and integrated GDK port updates. 
> 
> Please check Bakefiles/Readme.txt and ChangeLogs to learn more about this
> patch. Comments and suggestions welcome! Thanks!
> 

Sorry, I did forget to make one small change to the Readme.txt docs. You no longer need to specify the filename to make on Unix. Just running 'make' works fine.
Comment 16 Darin Adler 2006-04-24 20:43:27 PDT
Comment on attachment 7934 [details]
Renamed GNU makefiles to GNUmakefile and integrated GDK port updates. 

Why are we checking in generated projects and makefiles? We usually do not check in generated files.

It seems strange to have the bakefiles in a Bakefiles directory instead of inside the individual project directories.
Comment 17 Kevin Ollivier 2006-04-24 21:29:21 PDT
(In reply to comment #16)
> (From update of attachment 7934 [details] [edit])
> Why are we checking in generated projects and makefiles? We usually do not
> check in generated files.

I was just concerned that some people who are used to checkout -> build would not like the process to become checkout -> generate projects -> build. (e.g. if we ever moved the VC2005 projects over to using this system) The other scripts to create generated files are run automatically via the project files, but obviously we can't do that for the generation of the project files themselves, which means we add a manual step which would have to be run after each update. But at the same time, I see the logic for not keeping generated files in SVN, so I'm open to doing it either way.

If you want, I can go ahead and remove those generated files from the patch.

> It seems strange to have the bakefiles in a Bakefiles directory instead of
> inside the individual project directories.

The main issue is that several of those Bakefiles are shared between WebCore ports, and presets.bkl is shared between JavaScriptCore and WebCore, so coming up with 'appropriate' placement of shared items seemed more involved than just having one directory for all the Bakefiles. Of course, that could just be my organization style. :-) Did you want me to move the files into various directories? If so, do you have any suggestions as to where would be a good location for the presets.bkl and readme.txt files? 

Thanks!
Comment 18 Vaclav Slavik 2006-04-24 22:35:33 PDT
> obviously we can't do that for the generation of the project files themselves,
> which means we add a manual step which would have to be run after each update.

You could a check to makefiles, so that they are regenerated when they are older than the source bakefiles -- then this step would be manual only once. Obviously, this wouldn't work for project files, the best thing you can hope for here is a check in prebuild step that causes an error if the project file is older than the bakefiles.
Comment 19 Darin Adler 2006-04-27 07:41:03 PDT
Comment on attachment 7934 [details]
Renamed GNU makefiles to GNUmakefile and integrated GDK port updates. 

(In reply to comment #17)
> I was just concerned that some people who are used to checkout -> build would
> not like the process to become checkout -> generate projects -> build. (e.g. if
> we ever moved the VC2005 projects over to using this system) The other scripts
> to create generated files are run automatically via the project files, but
> obviously we can't do that for the generation of the project files themselves,
> which means we add a manual step which would have to be run after each update.
> But at the same time, I see the logic for not keeping generated files in SVN,
> so I'm open to doing it either way.
> 
> If you want, I can go ahead and remove those generated files from the patch.

Yes, I'd like to see this done without checking in the generated files.

People who want to just be able to check out and build can use a build script of some sort, and we can automate the checking at least as suggested by Vaclav Slavik -- for some build systems it can be entirely automatic, for the others it can at least be an error that is detected. Over time we may find ways to automate even for things like Microsoft Visual Studio projects.

> > It seems strange to have the bakefiles in a Bakefiles directory instead of
> > inside the individual project directories.
> 
> The main issue is that several of those Bakefiles are shared between WebCore
> ports, and presets.bkl is shared between JavaScriptCore and WebCore, so coming
> up with 'appropriate' placement of shared items seemed more involved than just
> having one directory for all the Bakefiles. Of course, that could just be my
> organization style. :-) Did you want me to move the files into various
> directories? If so, do you have any suggestions as to where would be a good
> location for the presets.bkl and readme.txt files? 

The files that list all the things in a project should be inside that project. For example, if there's a list of all the files in WebCore, it's very important that's inside the WebCore directory.

If there are shared things, it's fine for them to be in a shared place, but anything that has to change as a particular project changes should be in that project if possible.

Could you take another pass at this given those comments?
Comment 20 Kevin Ollivier 2006-04-29 22:03:43 PDT
Created attachment 8042 [details]
Moved bkl files, removed gen. files, and added MSVS 2005 support

Please let me know if this works for you, or if you want the Bakefiles folder completely gone. Generated files are gone from the tree. Also, I added MSVS 2005 support as it greatly simplifies the Windows build process and removes the need to up-convert. Thanks!
Comment 21 Darin Adler 2006-05-01 16:22:43 PDT
Comment on attachment 8042 [details]
Moved bkl files, removed gen. files, and added MSVS 2005 support

Even though I don't understand all the details, this looks good. Lets get this in there. r=me
Comment 22 Michael Emmel 2006-05-01 16:42:10 PDT
(In reply to comment #21)
> (From update of attachment 8042 [details] [edit])
> Even though I don't understand all the details, this looks good. Lets get this
> in there. r=me
> 

Cool once its in I'll send my latest patch with all the linux sources for building.
I was trying to fix bugs while this was still out :)
Comment 23 Kevin Ollivier 2006-05-11 22:08:07 PDT
Created attachment 8259 [details]
Needed to update to add XPath support and because WTF happened
Comment 24 Michael Emmel 2006-05-11 22:16:02 PDT
I've been waiting on this patch to submit my linux port. Hopefully it can go in soon.

Mike
Comment 25 Timothy Hatcher 2006-05-13 10:27:57 PDT
Landed in r14359