546456 - venkman icons shouldn't be installed when venkman is disabled

Status: RESOLVED WONTFIX

(SeaMonkey :: Build Config, defect)

Description

[Mike Hommey [:glandium]]

Attachment: Patch

When venkman is disabled through e.g. --with-extensions="default,-venkman", the venkman icons shouldn't be installed.

Comment 1

[Mike Hommey [:glandium]]

Attachment: Patch v2

The previous patch didn't end up doing the right thing because MOZ_EXTENSIONS is only defined in mozilla/config/autoconf.mk

Comment 2

[Serge Gautherie (:sgautherie)]

I don't think we want to (ever) include mozilla/config/autoconf.mk.

http://mxr.mozilla.org/comm-central/search?string=MOZ_EXTENSIONS&case=on
Interestingly, */locales/Makefile.in have a similar check...
You should do whatever is needed to make that variable available from the c-c build system.

Comment 3

[Mike Hommey [:glandium]]

(In reply to comment #2)
> I don't think we want to (ever) include mozilla/config/autoconf.mk.
> 
> http://mxr.mozilla.org/comm-central/search?string=MOZ_EXTENSIONS&case=on
> Interestingly, */locales/Makefile.in have a similar check...
> You should do whatever is needed to make that variable available from the c-c
> build system.

The only way this would work would be to copy parts of mozilla/configure.in that handle MOZ_EXTENSIONS. Duplicating code is bad and error-prone.

Comment 4

[Robert Kaiser]

Comment on attachment 427169 [details] [diff] [review]
Patch v2

No, we don't include mozilla/config/autoconf.mk

Also, branding is to be largely changed in bug 525869, which I'd like to not bitrot.

And lastly, I don't see too much use in this whole bug, actually. It doesn't do anything bad to have an icon installed that you just don't use.

Comment 5

[Mike Hommey [:glandium]]

Actually, the venkman icons should simply not been part of the seamonkey
branding, but part of mozilla/extensions/venkman, a bit like
mozilla/extensions/irc which contains the chatzilla icons (although the
branding also has them).

Comment 6

[Robert Kaiser]

Possibly, but OTOH, they need to be part of SeaMonkey branding if we want to have consistent branding over all the parts we ship by default. In any case, we have things like they are, so let's leave that.
Or you can file a bug on venkman and get venkman to ship icons of their own if that's supposed to be better (I'm somewhat unsure).

I don't see any actual problem we can solve right here though, and I mostly care about what we are actually shipping - which includes venkman in any case.

Comment 7

[Justin Wood (:Callek)]

Mike thank you for caring about us... 

but... in this particular case I don't see it as a problem.

Few potentials:

(1) We build normally, we want consistent branding = NOW
(2) We build without venkman, but have a relatively small icon in app folder that goes unused; no harm. = What is proposed to "fix" here.
(3) We build without venkman and user wants to actually INSTALL venkman as an extension (or has it _already_ installed as an extension when they run the app version without it build); IMO we should keep the icon for this case too.

Overall I'd support a wontfix here.

Comment 8

[Justin Wood (:Callek)]

No arguments to my c#7 --> WONTFIX