Bug#821270: Review of firefox-branding-iceweasel

[Charles Plessy]

Le Tue, Apr 19, 2016 at 01:46:46PM -0700, Sean Whitton a écrit :
> 
> "Every package must be accompanied by a verbatim copy of its copyright
> information and distribution license in the file
> /usr/share/doc/package/copyright."
> 
> It then makes an *exception* to this verbatim rule:
> 
> "Packages distributed under the Apache license (version 2.0), the
> Artistic license, the GNU GPL (versions 1, 2, or 3), the GNU LGPL
> (versions 2, 2.1, or 3), and the GNU FDL (versions 1.2 or 1.3) should
> refer to the corresponding files under /usr/share/common-licenses,
> rather than quoting them in the copyright file."
> 
> Since you are not using /usr/share/common-licenses, your package doesn't
> fall under this exception and so you need to include it in your
> d/copyright file.

Hi all,

actually, the MPL-2 will be added to /user/share/common-licenses when a Policy
Editor will find time to make it happen.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=768292

So I suggest that it is fine to be forward-compatible with the future Policy :)

Have a nice day,

-- 
Charles Plessy
Tsurumi, Kanagawa, Japan

Bug#821270: Review of firefox-branding-iceweasel

[Sean Whitton]

Hello,

On Wed, Apr 20, 2016 at 10:47:05AM +0800, Paul Wise wrote:
> On Wed, Apr 20, 2016 at 4:46 AM, Sean Whitton wrote:
> 
> > Yes, it should definitely be xul-ext-iceweasel-branding -- that's
> > pkg-mozext policy for anything that appears in about:addons.  dh_xul-ext
> > assumes that your package is called xul-ext-foo, and it will generate a
> > Provides: entry for firefox-foo.
> 
> The package name was suggested by Mozilla folks:
> 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815006#145

As far as I can see the person making that suggestion is not a part of
the pkg-mozext team, though.  And it's in the (draft) group policy:
https://wiki.debian.org/Mozilla/ExtensionsPolicy

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#821270: Review of firefox-branding-iceweasel

[Paul Wise]

On Wed, Apr 20, 2016 at 4:46 AM, Sean Whitton wrote:

> Yes, it should definitely be xul-ext-iceweasel-branding -- that's
> pkg-mozext policy for anything that appears in about:addons.  dh_xul-ext
> assumes that your package is called xul-ext-foo, and it will generate a
> Provides: entry for firefox-foo.

The package name was suggested by Mozilla folks:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815006#145

> How about adding a new file /usr/share/applications/iceweasel.desktop
> that launches firefox with the old icon?  This would make this extension
> conflict with iceweasel, but I think that would be okay so long as you
> added a Conflicts: line in d/control.

I think it would be best to handle this in the iceweasel package instead.

-- 
bye,
pabs

https://wiki.debian.org/PaulWise

Bug#821270: Review of firefox-branding-iceweasel

[Sean Whitton]

Hello,

On Tue, Apr 19, 2016 at 02:51:35PM +, nord-stream wrote:
> I thought of creating packages named firefox-branding-iceweasel,
> thunderbird-branding-icedove, etc. I am aware of the naming convention,
> but these extensions are not much like extensions but just branding
> packages. (In fact it Provides: xul-ext-iceweasel-branding.)
> Is that naming mandatory? I also found many of xul-ext-* packages do not
> include a single XUL file. (Neither does firefox-branding-iceweasel.)
> More discussion at:
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815006#145

Yes, it should definitely be xul-ext-iceweasel-branding -- that's
pkg-mozext policy for anything that appears in about:addons.  dh_xul-ext
assumes that your package is called xul-ext-foo, and it will generate a
Provides: entry for firefox-foo.

> > - don't install the MPL-* files using debian/docs.  Instead, include
> > the full license text in debian/copyright.
> 
> firefox-esr package seems to do this. Do you mean that it is not
> appropriate for a branding package?

I'm pretty sure that firefox-esr is wrong to do that.  Policy 12.5 says:

"Every package must be accompanied by a verbatim copy of its copyright
information and distribution license in the file
/usr/share/doc/package/copyright."

It then makes an *exception* to this verbatim rule:

"Packages distributed under the Apache license (version 2.0), the
Artistic license, the GNU GPL (versions 1, 2, or 3), the GNU LGPL
(versions 2, 2.1, or 3), and the GNU FDL (versions 1.2 or 1.3) should
refer to the corresponding files under /usr/share/common-licenses,
rather than quoting them in the copyright file."

Since you are not using /usr/share/common-licenses, your package doesn't
fall under this exception and so you need to include it in your
d/copyright file.

Further the machine-readacble copyright specification says "... this
field should either include the full text of the license(s) or include a
pointer to the license file under /usr/share/common-licenses."

> > - on my machine, the package doesn't change the application icon --
> >   see the top of the attached screenshot.  Maybe you can't fix that,
> >   though.
> 
> Not possible with an extension. Doable with a .desktop file, I think.

How about adding a new file /usr/share/applications/iceweasel.desktop
that launches firefox with the old icon?  This would make this extension
conflict with iceweasel, but I think that would be okay so long as you
added a Conflicts: line in d/control.

> The complex part of Makefile is from Iceweasel package. Although most
> extensions' Makefiles just pack files into .xpi, it generates files from
> source files. This tiny override just saved me of hours of studying more
> about customizing dh_xul-ext.

Indeed: most of your Makefile complexity is unavoidable.

However, some reasons why it would be good if you avoided the override:

- it makes it easier for team members working on dh_xul-ext to know
  whether some change they are working on will break this package; and

- more generally, it makes it easier for team members to work on your
  package if files have the standard layout (that's the point of team
  maintenance).

It's not just about a short debian/rules: it makes your package more
standardised overall which is a good thing for your project :)

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#821270: Review of firefox-branding-iceweasel

[Gianfranco Costamagna]

Hi,
(just answering the alioth part)
https://qa.debian.org/developer.php?login=pkg-mozext-maintain...@lists.alioth.debian.org
this should be the maintainer

and the account has to be created on alioth.d.o, and then join the group
https://alioth.debian.org/projects/pkg-mozext

cheers,

G.




Il Martedì 19 Aprile 2016 16:51, nord-stream  ha 
scritto:
On 18/04/16 20:20, Sean Whitton wrote:
> The package is mostly fine.  Here are some points:
>
> - binary package name should be xul-ext-iceweasel-branding or similar

I thought of creating packages named firefox-branding-iceweasel,
thunderbird-branding-icedove, etc. I am aware of the naming convention,
but these extensions are not much like extensions but just branding
packages. (In fact it Provides: xul-ext-iceweasel-branding.)
Is that naming mandatory? I also found many of xul-ext-* packages do not
include a single XUL file. (Neither does firefox-branding-iceweasel.)
More discussion at:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815006#145

> - is it possible to generalise this to restore both icedove and
>   iceweasel branding in one binary package?  (icedove will soon become
>   thunderbird)

It may be possible, but many people use one and not another, so one
binary package may not be desirable. One source package may be a good
idea, though.

> - don't install the MPL-* files using debian/docs.  Instead, include 
>   the full license text in debian/copyright.

firefox-esr package seems to do this. Do you mean that it is not
appropriate for a branding package?

> - as Gianfranco suggested, this should be team maintained.  Your name
>   should be in the Uploaders: field in debian/control, and Maintainer:
>   should be the Mozilla extensions packaging team.  You should upload
>   the git repository to the Mozilla extensions team section of alioth.

That seems right. Please tell me more.

>   Do you have an alioth account?

I don't. What does it mean? I'm willing to do what's needed.

> - the long description is not, IMO, appropriate.  You should include
>   the history of the package in the README.markdown, and just give a
>   terse description of what it does in the long description (or at
>   least in the first paragraph of the long description)

I'll consider this point later.

> - on my machine, the package doesn't change the application icon --
>   see the top of the attached screenshot.  Maybe you can't fix that,
>   though.

Not possible with an extension. Doable with a .desktop file, I think.

On 19/04/16 02:18, Sean Whitton wrote:
> I just took a closer look at your debian/rules file.  You don't need the
> boilerplate.  This is enough:

Done. Will upload later.


> However, since this is a native package, would you consider editing your
> Makefile so that dh_xul-ext can do your whole build for you?  Take a
> look at the source package y-u-no-validate.  It uses this main Makefile
> target:
> 
> ,
> | %:
> | dh $@ --with xul-ext --buildsystem=xul_ext --sourcedirectory=src
> `
> 
> There is an override, but it's just something minor.  This works because
> the source code is organised in a standard way, but your code seems to
> be organised in a non-standard way which is why you need a complex
> Makefile and dh overrides.

The complex part of Makefile is from Iceweasel package. Although most
extensions' Makefiles just pack files into .xpi, it generates files from
source files. This tiny override just saved me of hours of studying more
about customizing dh_xul-ext.

Thank you for reviewing,

--
nord-stream

Bug#821270: Review of firefox-branding-iceweasel

[nord-stream]

On 18/04/16 20:20, Sean Whitton wrote:
> The package is mostly fine.  Here are some points:
>
> - binary package name should be xul-ext-iceweasel-branding or similar

I thought of creating packages named firefox-branding-iceweasel,
thunderbird-branding-icedove, etc. I am aware of the naming convention,
but these extensions are not much like extensions but just branding
packages. (In fact it Provides: xul-ext-iceweasel-branding.)
Is that naming mandatory? I also found many of xul-ext-* packages do not
include a single XUL file. (Neither does firefox-branding-iceweasel.)
More discussion at:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=815006#145

> - is it possible to generalise this to restore both icedove and
>   iceweasel branding in one binary package?  (icedove will soon become
>   thunderbird)

It may be possible, but many people use one and not another, so one
binary package may not be desirable. One source package may be a good
idea, though.

> - don't install the MPL-* files using debian/docs.  Instead, include 
>   the full license text in debian/copyright.

firefox-esr package seems to do this. Do you mean that it is not
appropriate for a branding package?

> - as Gianfranco suggested, this should be team maintained.  Your name
>   should be in the Uploaders: field in debian/control, and Maintainer:
>   should be the Mozilla extensions packaging team.  You should upload
>   the git repository to the Mozilla extensions team section of alioth.

That seems right. Please tell me more.

>   Do you have an alioth account?

I don't. What does it mean? I'm willing to do what's needed.

> - the long description is not, IMO, appropriate.  You should include
>   the history of the package in the README.markdown, and just give a
>   terse description of what it does in the long description (or at
>   least in the first paragraph of the long description)

I'll consider this point later.

> - on my machine, the package doesn't change the application icon --
>   see the top of the attached screenshot.  Maybe you can't fix that,
>   though.

Not possible with an extension. Doable with a .desktop file, I think.

On 19/04/16 02:18, Sean Whitton wrote:
> I just took a closer look at your debian/rules file.  You don't need the
> boilerplate.  This is enough:

Done. Will upload later.

> However, since this is a native package, would you consider editing your
> Makefile so that dh_xul-ext can do your whole build for you?  Take a
> look at the source package y-u-no-validate.  It uses this main Makefile
> target:
> 
> ,
> | %:
> | dh $@ --with xul-ext --buildsystem=xul_ext --sourcedirectory=src
> `
> 
> There is an override, but it's just something minor.  This works because
> the source code is organised in a standard way, but your code seems to
> be organised in a non-standard way which is why you need a complex
> Makefile and dh overrides.

The complex part of Makefile is from Iceweasel package. Although most
extensions' Makefiles just pack files into .xpi, it generates files from
source files. This tiny override just saved me of hours of studying more
about customizing dh_xul-ext.

Thank you for reviewing,

--
nord-stream



signature.asc
Description: OpenPGP digital signature

Bug#821270: Review of firefox-branding-iceweasel

[Sean Whitton]

Hello,

I just took a closer look at your debian/rules file.  You don't need the
boilerplate.  This is enough:

,
| #!/usr/bin/make -f
| 
| %:
|   dh $@ --with xul-ext --parallel
| 
| override_dh_auto_install:
|   xpi-pack ./xpi-build ./build.xpi
|   install-xpi ./build.xpi
`

However, since this is a native package, would you consider editing your
Makefile so that dh_xul-ext can do your whole build for you?  Take a
look at the source package y-u-no-validate.  It uses this main Makefile
target:

,
| %:
|   dh $@ --with xul-ext --buildsystem=xul_ext --sourcedirectory=src
`

There is an override, but it's just something minor.  This works because
the source code is organised in a standard way, but your code seems to
be organised in a non-standard way which is why you need a complex
Makefile and dh overrides.

-- 
Sean Whitton


signature.asc
Description: PGP signature

Bug#821270: Review of firefox-branding-iceweasel

[Sean Whitton]

The package is mostly fine.  Here are some points:

- binary package name should be xul-ext-iceweasel-branding or similar

- is it possible to generalise this to restore both icedove and
  iceweasel branding in one binary package?  (icedove will soon become
  thunderbird)

- don't install the MPL-* files using debian/docs.  Instead, include the
  full license text in debian/copyright.

- as Gianfranco suggested, this should be team maintained.  Your name
  should be in the Uploaders: field in debian/control, and Maintainer:
  should be the Mozilla extensions packaging team.  You should upload
  the git repository to the Mozilla extensions team section of alioth.
  Do you have an alioth account?

- the long description is not, IMO, appropriate.  You should include the
  history of the package in the README.markdown, and just give a terse
  description of what it does in the long description (or at least in
  the first paragraph of the long description)

- on my machine, the package doesn't change the application icon -- see
  the top of the attached screenshot.  Maybe you can't fix that, though.

-- 
Sean Whitton


signature.asc
Description: PGP signature