Closed Bug 423277 Opened 13 years ago Closed 4 years ago

xulrunner+linux basic embedding profile packaging update

Categories

(Firefox Build System :: General, defect)

x86
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: dougt, Unassigned)

Details

Attachments

(2 files, 7 obsolete files)

make package should package up the 'right' stuff for linux/basic.  This bug is specific to the xulrunner basic embedding profile on linux.
this shouldn't change anything except on the basic embedding profile on gtk2.
Assignee: nobody → doug.turner
Status: NEW → ASSIGNED
Attachment #309772 - Flags: superreview?
Attachment #309772 - Flags: review?
this needs to be in : xulrunner/installer/gtk2/packages-basic.


Not sure about why I need ./run-mozilla.sh.  Without it, running ./xulrunner doesn't work.
Attachment #309773 - Flags: review?
Attachment #309772 - Flags: superreview?(benjamin)
Attachment #309772 - Flags: superreview?
Attachment #309772 - Flags: review?(benjamin)
Attachment #309772 - Flags: review?
Attachment #309773 - Flags: review? → review?(benjamin)
Attached file file listing of dist/xulrunner (obsolete) —
these are all of the files in dist/xulrunner after running make package.


It would be cool to rename "browser.xpt" to something else -- like "xulrunner.xpt"
Comment on attachment 309772 [details] [diff] [review]
installer makefile.  sets package manifest on gtk2

You set MOZ_PKG_MANIFEST = MOZ_PKG_MANIFEST_P, which will cause you to preprocess packages-basic back into the srcdir.

Also, why do you need PREF_DIR? Firefox needs it because FF+XR is different, but XR shouldn't need any special defines there.

New code should use preprocessor.py, not preprocessor.pl
Attachment #309772 - Flags: superreview?(benjamin)
Attachment #309772 - Flags: review?(benjamin)
Attachment #309772 - Flags: review-
Attachment #309772 - Attachment is obsolete: true
Attachment #309984 - Flags: review?
Attachment #309984 - Attachment is patch: true
Attachment #309984 - Attachment mime type: application/octet-stream → text/plain
Attachment #309984 - Flags: review? → review?(benjamin)
Comment on attachment 309984 [details] [diff] [review]
patch to Makefile.in (addresses bsmedberg comments)

Does this work? You don't seem to be setting MOZ_PKG_MANIFEST anywhere.
Attachment #309984 - Flags: review?(benjamin) → review-
Attached patch cleaner patch (obsolete) — Splinter Review
sure it worked ;-)  but this is probably cleaner.  I removed the *_P variable.
Attachment #309984 - Attachment is obsolete: true
Attachment #313630 - Flags: review?
Comment on attachment 313630 [details] [diff] [review]
cleaner patch

>Index: Makefile.in

>+ifdef MOZ_EMBEDDING_LEVEL_BASIC
>+ifdef MOZ_ENABLE_GTK2
>+MOZ_PKG_MANIFEST = $(srcdir)/gtk2/packages-basic
>+endif
>+endif

With this patch, packager.mk will use packages-basic from the srcdir without any preprocessing. I don't think you want that. What you do want, I think, is

MOZ_PKG_MANIFEST = packages-basic

>+ifdef MOZ_PKG_MANIFEST
>+DEFINES += -DAB_CD=$(AB_CD) -DMOZ_APP_NAME=$(MOZ_APP_NAME)
>+
>+$(MOZ_PKG_MANIFEST):
>+	$(PERL) $(topsrcdir)/config/preprocessor.py $(DEFINES) $(ACDEFINES) $< > $@
>+endif

This target still doesn't make sense: you wrote $< in a rule which has no prerequisites. I think you want:

$(MOZ_PKG_MANIFEST):
	$(PERL) $(topsrcdir)/config/preprocessor.py $(DEFINES) $(ACDEFINES) $(srcdir)/gtk2/packages-basic > $@
Attachment #313630 - Flags: review? → review-
how will:
MOZ_PKG_MANIFEST = packages-basic

pull from the right subdirectory?

eventually, we want something like:

srcdir/mac/packages-basic
srcdir/gtk2/packages-basic
srcdir/qt/packages-basic

as there are different bits that are platform specific.
There are two steps:

1) preprocess the platform-specific manifest into the objdir.
2) tell packager.mk where the manifest is in the objdir

MOZ_PKG_MANIFEST is the latter step

you have to write a custom rule to do #1, and you can use any magic you want for that part.
Attached patch patch v.2 (obsolete) — Splinter Review
Attachment #313630 - Attachment is obsolete: true
Attached file package list (obsolete) —
Attachment #309773 - Attachment is obsolete: true
Attachment #309773 - Flags: review?(benjamin)
Comment on attachment 314168 [details] [diff] [review]
patch v.2

should be using python, not perl.
Attachment #314168 - Attachment is obsolete: true
Attached patch patch v.3Splinter Review
Attached file packages-basic file
Attachment #309774 - Attachment is obsolete: true
Attachment #314410 - Attachment is obsolete: true
Attachment #314672 - Flags: review?(benjamin)
Comment on attachment 314672 [details] [diff] [review]
patch v.3

>Index: xulrunner/installer/Makefile.in

>+$(MOZ_PKG_MANIFEST): packages-basic
>+	$(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $(ACDEFINES) $(srcdir)/gtk2/packages-basic > $@

The dependency doesn't make sense. You should have:

$(MOZ_PKG_MANIFEST): gtk2/packages-basic
	$(PYTHON) $(topsrcdir)/config/Preprocessor.py $(DEFINES) $(ACDEFINES) $< > $@

r=me with that change
Attachment #314672 - Flags: review?(benjamin) → review+
Flags: blocking1.9?
This shouldn't block 1.9.  Not something we'll reject, but won't block the release.  Re-nom if you disagree and state why.
Flags: blocking1.9? → blocking1.9-
would like to land this patch somewhere, doesn't effect firefox dev.
Flags: blocking1.9- → blocking1.9?
Feel free to request approval.  :)
Flags: blocking1.9? → blocking1.9-
Attachment #309772 - Flags: approval1.9?
Attachment #309772 - Flags: approval1.9?
per mobile call, i am not sure we are interested in a basic profile at the moment.  
over to default.
Assignee: doug.turner → nobody
Status: ASSIGNED → NEW
I'm going to say this is no longer relevant.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.