329686 - update-packaging tools need their own target

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect, P3)

Description

[J. Paul Reed [:preed]]

Right now, to get the tools necessary to create/manipulate mar files, either a) a full build must be done, or b) you must know which directories need to be compiled, in the right order, to create the two necessary tools, mbsdiff and mar (they typically live in dist/host/bin).

A target needs to be created so you can pull the relevant subset of the tree, and make a target to build these two utilities and that's it.

Comment 1

[J. Paul Reed [:preed]]

Attachment: Create a tools/update-packaging target

This creates an --enable-application=tools/update-packaging target, much like the content/xslt target (per bsmedberg's request).

Tested by:

-- applying patch to a clean tree
-- Running autoconf
-- ./configure --enable-application=tools/update-packaging
-- make

Gives us:

[preed@preed-lx mozilla]$ ls dist/host/bin/
mar  mbsdiff  nsinstall

Which is what is wanted.

What this patch is missing: a proper checkout target, for client.mk.

Right now, checking out the browser mostly works; mozilla/other-licenses/mbsdiff is also needed (it's not pulled by default; maybe it should be?)

Comment 2

[Darin Fisher]

Comment on attachment 214373 [details] [diff] [review]
Create a tools/update-packaging target

Don't you need to add something to allmakefiles.sh?  Would be good to get bsmedberg to review this, but I know that he is out of town.  I suggest requesting review from him after-the-fact.

Comment 3

[Darin Fisher]

s/out of town/a new father/

Comment 4

[J. Paul Reed [:preed]]

(In reply to comment #2)
> (From update of attachment 214373 [details] [diff] [review] [edit])
> Don't you need to add something to allmakefiles.sh?  Would be good to get
> bsmedberg to review this, but I know that he is out of town.  I suggest
> requesting review from him after-the-fact.

I don't *think* it's always required, but I've never added a produt before, so maybe?

I notice that there were no additional entries added to allmakefiles.sh for the xslt move (Bug 304494), and this doesn't create any new Makefiles (in terms of new Makefile.in's).

Totally agree on bsmedberg reviewing, but this is semi-blocking (or at least, related to) bug 327140; there will likely be a second part to this, which is to create an appropriate client.mk CVS pull target for this stuff; we should be able to clean up any horribleness then.

Comment 5

[Darin Fisher]

> I don't *think* it's always required, but I've never added a produt before, so
> maybe?
> 
> I notice that there were no additional entries added to allmakefiles.sh for the
> xslt move (Bug 304494), and this doesn't create any new Makefiles (in terms of
> new Makefile.in's).

What about other-licenses/bsdiff/Makefile.in?  How does a Makefile get generated for that if we don't have a line in allmakefiles.sh?

Comment 6

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #5)

> What about other-licenses/bsdiff/Makefile.in?  How does a Makefile get
> generated for that if we don't have a line in allmakefiles.sh?

The patch in bug 321650 adds it.

Comment 7

[Darin Fisher]

Comment on attachment 214373 [details] [diff] [review]
Create a tools/update-packaging target

>Index: Makefile.in
...
>+ifeq ($(MOZ_BUILD_APP),content/xslt) # {
> DIRS = \
> 	xpcom/typelib \
> 	xpcom \
> 	parser/expat \
> 	content/xslt/src \
> 	$(NULL)
>+	
>+	SUBMAKEFILES = xpcom/typelib/Makefile
> 
>-SUBMAKEFILES = xpcom/typelib/Makefile

The indentation of SUBMAKEFILES here seems a little wierd to me.
The other assignments to that variable are not similarly indented.

r=darin

Comment 8

[J. Paul Reed [:preed]]

(In reply to comment #7)

> The indentation of SUBMAKEFILES here seems a little wierd to me.
> The other assignments to that variable are not similarly indented.

Good point; fixed in the checkin, which I just committed.

Next up will be a change to pull the correct directories.

Comment 9

[J. Paul Reed [:preed]]

Comment on attachment 214373 [details] [diff] [review]
Create a tools/update-packaging target

Per darin's suggestion, request an after-the-fact review from bsmedberg.

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

This patch has causes make to scan the entire tree for things to be done for 'tools'. Is that really needed? It takes quite some time...

Comment 11

[Benjamin Smedberg]

That's not this patch, it was from bug 299988, and yes it's necessary.

Comment 12

[Benjamin Smedberg]

Comment on attachment 214373 [details] [diff] [review]
Create a tools/update-packaging target

I'm not sold on the bracing style: it seems to me that the file is even less readable than it was.

The indent of SUBMAKEFILES is a little odd (and really shouldn't be necessary!)

Comment 13

[J. Paul Reed [:preed]]

(In reply to comment #12)

> The indent of SUBMAKEFILES is a little odd (and really shouldn't be necessary!)

That was fixed in the final checkin. My bad.

Comment 14

[J. Paul Reed [:preed]]

Reassigning bugs I'm not actively working on back into the triage pool.

Comment 15

[(not currently active) Ted Mielczarek]

May be more fixable given the work from bug 380846.

Comment 16

[Robert Helmer [:rhelmer]]

(In reply to comment #15)
> May be more fixable given the work from bug 380846.

What else is left? A checkout target which only checks out the needed dirs (instead of all)?

Comment 17

[(not currently active) Ted Mielczarek]

In our brave new Mercurial world, partial checkouts aren't possible any more. We could stick this stuff in its own Mercurial repo if its useful to have it standalone.

Comment 18

[Robert Strong (they/them - no direct email)]

Resolving fixed by preed's checkins since there hasn't been anyone requesting that these files go in their own repo and there were patches checked in that fixed the rest.