net-proxy/haproxy: Add support for DeviceAtlas API #2487

bjacquin · 2016-10-05T23:58:06Z

Bug: https://bugs.gentoo.org/show_bug.cgi?id=564160

doughdemon · 2016-10-06T03:08:12Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+HOMEPAGE="https://deviceatlas.com"
+SRC_URI="https://deviceatlas.com/download/api/enterprise/c -> ${MY_P}.zip"
+
+LICENSE="public-domain"


Is this really the correct license for this fetch-restricted package?

As a registration to the website is the only requirement to get the source code as direct redistribution is not allowed, would it be possible to get either this a custom licence text

License

The free of charge offering has a restricted use license, for own usage only; no redistribution or derivative works are permitted. Contact https://www.deviceatlas.com/contact-us to upgrade or to enable usage as part of a service offering.

or like the FreeBSD port

https://svnweb.freebsd.org/ports/head/net/haproxy/Makefile?revision=421150&view=markup

an equivalent to the RESTRICTED ?
Thanks.

@devnexen LICENSE is specified as "public-domain" since there is no explicit license in the source code or in the zip file available to download. I believe the license you are quoting will not be enough for legal purpose.

Fair enough then anyway by registering to the website you agree to the statement above. Otherwise the rest is very ok to me thanks !

Well my point was, would you mind adding an explicit license so we can replace public-domain ?

<hat type="Licenses">
I can't find any actual license text on their website either

all-rights-reserved is available in tree:
LICENSE=all-rights-reserved.
You also must add RESTRICT='fetch mirror bindist'
</hat>

@robbat2 https://deviceatlas.com/deviceatlas-haproxy-module

@devnexen I read that URL already; it's just instructions for building HAProxy really.

This is a description of a license, but is NOT a license text itself:

The free of charge offering has a restricted use license, for own usage only; no redistribution or derivative works are permitted.

Fair enough.

Fix in last PR, thanks for looking at this.

gktrk · 2016-10-08T09:03:58Z

@idl0r

monsieurp · 2016-10-21T08:56:48Z

ping

idl0r · 2016-10-21T14:46:56Z

I didn't get to it yet. I know there's a bug about it already and IIRC I even got a mail. I need to take some time to take a look at it. I am also not sure re licensing/restriction etc.

robbat2 · 2016-11-25T18:52:48Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+
+DESCRIPTION="API to detect devices based on the User-Agent HTTP header"
+HOMEPAGE="https://deviceatlas.com"
+SRC_URI="https://deviceatlas.com/download/api/enterprise/c -> ${MY_P}.zip"


As a manual-download-only distfile, this should be JUST:

SRC_URI="${MY_P}.zip"

Fixed in last PR. Thanks

mgorny · 2016-11-25T21:41:54Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+KEYWORDS="~amd64 ~arm ~ppc ~x86"
+IUSE="doc examples"
+
+RESTRICT="fetch"


Please put this below dependencies. While specific order of variables is not strictly requires, it helps other people read your ebuilds and reduces the risk that someone will miss something.

Fixed in last PR

mgorny · 2016-11-25T21:42:53Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+
+RESTRICT="fetch"
+
+CDEPEND="dev-libs/libpcre[${MULTILIB_USEDEP}]"


Please don't use one-letter dependency-like variables, to avoid potential collisions in the future. In this case, you don't need an additional variable, just put RDEPEND first and assing DEPEND from it.

Make sense, fixed in last PR

mgorny · 2016-11-25T21:44:01Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+}
+
+multilib_src_install_all() {
+	default


Strictly speaking, default is emake install+einstalldocs. The former you probably don't want here, the latter you do further on. So remove it.

Make sense, fixed in last PR

mgorny · 2016-11-25T21:44:22Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+	default
+
+	if use doc ; then
+		HTML_DOCS=( Documentation )


You can make this variable local.

Done in last PR

mgorny · 2016-11-25T21:44:41Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+
+	if use examples ; then
+		insinto /usr/share/doc/${P}
+		doins examples/daexutil.h


This is equivalent to dodoc ... since EAPI 5.

This was a typo, fixed in last PR. Thanks

mgorny · 2016-11-25T21:45:54Z

dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild

+		doins examples/daexutil.h
+		doins examples/example{0,1,2,3}.c
+		doins examples/util.c
+		DOC+=( examples/EXAMPLES.USAGE )


This variable is not used. You probably meant DOCS. However, you should note that appending to DOCS won't work as you expect it to, so you either need to list all the files or call dodoc explicitly. In fact, it'd probably make sense to just put one big dodoc call here.

This was part of the typo as well. Thanks for spotting this.

mgorny · 2016-11-25T21:46:44Z

dev-libs/device-atlas-api-c/metadata.xml

+		<email>bertrand@jacquin.bzh</email>
+		<name>Bertrand Jacquin</name>
+	</maintainer>
+	<longdescription>


Please add proxy-maint project here as well, unless you have some other developer who wants to commit for you.

Done in last PR

@bjacquin what last PR; I don't see a new PR with just dev-libs/device-atlas-api-c

mgorny · 2016-11-25T21:49:14Z

net-proxy/haproxy/files/1.6.9-MINOR-build-Allow-linking-to-device-atlas-library-fi.patch

+ DEVICEATLAS_SRC =
+ DEVICEATLAS_INC = $(DEVICEATLAS_SRC)
+ DEVICEATLAS_LIB = $(DEVICEATLAS_SRC)
+ifeq ($(DEVICEATLAS_LIB),)


This looks a bit suspicious to me. Do I understand correctly that the else branch was used to compile DeviceAtlas in place?

@bjacquin reping for this part. Also is this still needed for 1.6.10 / 1.7 releases?

ulm · 2016-11-26T07:53:48Z

@mgorny requested changes on this pull request. ------------------------------------------------------------------------------ In dev-libs/device-atlas-api-c/device-atlas-api-c-2.1.ebuild: +LICENSE="public-domain" +SLOT="0" +KEYWORDS="~amd64 ~arm ~ppc ~x86" +IUSE="doc examples" + +RESTRICT="fetch" Please put this below dependencies. While specific order of variables is not strictly requires, it helps other people read your ebuilds and reduces the risk that someone will miss something.

Huh? RESTRICT belongs in the second variable block (namely LICENSE, SLOT, KEYWORDS, IUSE, RESTRICT), see skel.ebuild. Especially, LICENSE and RESTRICT should stay close together, not be separated by a potentially long list of dependencies. So the only issue I see here is that the empty line before RESTRICT should be deleted. (And of course, "bindist" must be added as mentioned earlier.)

robbat2

I have implemented the HAProxy parts for Device indentification: DeviceAtlas, 51Degrees, WURFL; please abandon this PR and generate a new PR for just dev-libs/device-atlas-api-c

…Agent HTTP header Package-Manager: portage-2.3.0

Bug: https://bugs.gentoo.org/show_bug.cgi?id=564160 Package-Manager: portage-2.3.0

gentoo-repo-qa-bot · 2017-01-11T02:32:09Z

👍 All QA issues have been fixed!

doughdemon reviewed Oct 6, 2016

View reviewed changes

gktrk added the assigned PR successfully assigned to the package maintainer(s). label Oct 8, 2016

bjacquin force-pushed the dev/beber/haproxy-deviceatlas branch from a7faa29 to 1f2920b Compare November 13, 2016 23:56

mgorny added the help wanted The submitter explicitly needs help moving the PR forward. Please help if you can. label Nov 25, 2016

robbat2 reviewed Nov 25, 2016

View reviewed changes

mgorny requested changes Nov 25, 2016

View reviewed changes

mgorny added new package The PR is adding a new package. and removed help wanted The submitter explicitly needs help moving the PR forward. Please help if you can. labels Nov 25, 2016

robbat2 requested changes Dec 5, 2016

View reviewed changes

bjacquin force-pushed the dev/beber/haproxy-deviceatlas branch 2 times, most recently from 9f97317 to f4c523f Compare December 14, 2016 00:24

mgorny approved these changes Dec 14, 2016

View reviewed changes

bjacquin force-pushed the dev/beber/haproxy-deviceatlas branch from f4c523f to c24b89e Compare January 11, 2017 01:48

bjacquin added 2 commits January 11, 2017 02:02

dev-libs/device-atlas-api-c: API to detect devices based on the User-…

b9475e9

…Agent HTTP header Package-Manager: portage-2.3.0

net-proxy/haproxy: Add support for DeviceAtlas API

e25fbc2

Bug: https://bugs.gentoo.org/show_bug.cgi?id=564160 Package-Manager: portage-2.3.0

bjacquin force-pushed the dev/beber/haproxy-deviceatlas branch from c24b89e to e25fbc2 Compare January 11, 2017 02:03

gentoo-bot closed this in 62a9211 Jan 22, 2017

bjacquin deleted the dev/beber/haproxy-deviceatlas branch January 28, 2017 14:44


		RESTRICT="fetch"

		CDEPEND="dev-libs/libpcre[${MULTILIB_USEDEP}]"

net-proxy/haproxy: Add support for DeviceAtlas API #2487

net-proxy/haproxy: Add support for DeviceAtlas API #2487

Conversation

bjacquin commented Oct 5, 2016 • edited

Choose a reason for hiding this comment

devnexen Oct 6, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gktrk commented Oct 8, 2016

monsieurp commented Oct 21, 2016

idl0r commented Oct 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulm commented Nov 26, 2016 via email

robbat2 left a comment

Choose a reason for hiding this comment

gentoo-repo-qa-bot commented Jan 11, 2017

bjacquin commented Oct 5, 2016 •

edited

devnexen Oct 6, 2016 •

edited