Closed Bug 290490 Opened 20 years ago Closed 20 years ago

integrate suite/ into build process

Categories

(SeaMonkey :: Build Config, defect)

defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: kairo, Assigned: kairo)

Details

Attachments

(1 file, 1 obsolete file)

Now that the suite/ module is there, we should integrate it into the build process and pull it. Patch coming up in a moment.
Attached patch patch: add suite/ to build (obsolete) — Splinter Review
OK, I hope this patch does what we want. I also introduced a MOZ_SEAMONKEY define for including suite-specific stuff, so we can also define MOZ_XUL_APP one time and don't get bitten by suddely excluding our stuff ;-)
Attachment #180815 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #180815 - Flags: review?(chase)
maybe the define should be MOZ_SUITE instead?
(In reply to comment #2) > maybe the define should be MOZ_SUITE instead? Actually, I have no feelings about that, I just followed the example of the other apps we have... Not sure what is The Right Thing (tm) to do here...
Comment on attachment 180815 [details] [diff] [review] patch: add suite/ to build Since the toolkit apps do not wish to pull suite/, we need to fix the MODULES_toolkit stuff. Probably have a MODULES_core and make MODULES_suite extend that. In addition, we should not be coding MOZ_SEAMONKEY. Try MOZ_SUITE instead? Lastly, adding MOZ_SEAMONKEY=1 to configure.in doesn't help at all unless you AC_DEFINE it.
Attachment #180815 - Flags: review?(chase) → review-
I would personally prefer (as well) to keep the generic (Suite) where possible rather than a product/project name. Just makes more sense to me, though I am not sure if that will conflict with anything. And I will accept whatever the final choice is regardless.
OK, this changes to MOZ_SUITE, and introduces MODULES_core to client.mk (which is exactly what MOZDULES_suite was until now), so that we don't have to include the whole suite into toolkit et al. The AC_DEFINE is now there as well, thanks for spotting it :)
Attachment #180815 - Attachment is obsolete: true
Attachment #180824 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #180824 - Flags: review?(benjamin)
Comment on attachment 180815 [details] [diff] [review] patch: add suite/ to build >+DIRS = >+ Shouldn't this be DIRS = \ $(NULL) so as to avoid having to modify that line in future? moa=me with this and whatever it is bsmedberg wants fixed.
Attachment #180815 - Attachment is obsolete: false
Attachment #180815 - Flags: superreview?(neil.parkwaycc.co.uk)
Comment on attachment 180824 [details] [diff] [review] patch v2: address bsmedberg's comments Are you planning on landing this patch before you've got anything more in mozilla/suite ? If so, you should remove the DIRS= line in that file until you actually have subdirectories to build.
Attachment #180824 - Flags: review?(benjamin) → review+
Attachment #180824 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(In reply to comment #8) > (From update of attachment 180824 [details] [diff] [review] [edit]) > Are you planning on landing this patch before you've got anything more in > mozilla/suite ? If so, you should remove the DIRS= line in that file until you > actually have subdirectories to build. OK, good. Yes, I'm actually planning to land it so that someone putting real stuff in suite/ (e.g. gandalf with locales, or anyone working on the toolkit installer - our two first things targeting suite/) doesn't need to care about how to get the dir itself into the build process, but only has to care about what's happening in suite/ itself. It doesn't harm us to step in and build nothing right now, but it eases the process for someone who puts stuff in there.
Attachment #180815 - Attachment is obsolete: true
Comment on attachment 180824 [details] [diff] [review] patch v2: address bsmedberg's comments requesting approval: this adds the new, but still empty suite/ dir to the suite build process only.
Attachment #180824 - Flags: approval1.8b2?
Comment on attachment 180824 [details] [diff] [review] patch v2: address bsmedberg's comments a=asa
Attachment #180824 - Flags: approval1.8b2? → approval1.8b2+
checked in.
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Just a note, the checkin reading bug 290490 - editorOverlay inserts a second menupopup into message compose view show/hide menu, patch by Justin Wood <bugspam.Callek@gmail.com> r+sr=Neil a=asa Should really point to Bug 289366. Marking here so blame can still be useful for those changes.
Verified FIXED; I build on Windows XP all the time with no problem.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: