Page MenuHomeFreeBSD

Upgrade devel/protobuf-c to version 1.1.0
ClosedPublic

Authored by truckman on Jan 6 2015, 8:24 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sep 28 2024, 7:36 AM
Unknown Object (File)
Sep 12 2024, 2:30 AM
Unknown Object (File)
Sep 12 2024, 2:25 AM
Unknown Object (File)
Sep 9 2024, 1:05 AM
Unknown Object (File)
Sep 7 2024, 9:23 PM
Unknown Object (File)
Sep 2 2024, 1:53 AM
Unknown Object (File)
Sep 1 2024, 7:17 AM
Unknown Object (File)
Aug 31 2024, 8:57 PM
Subscribers
None

Details

Reviewers
mat
Summary

Upgrade to version 1.1.0:

protobuf-c (1.1.0)

[ Robert Edmonds ]
* Release 1.1.0.

[ Ilya Lipnitskiy ]
* Fix a bug when merging optional byte fields.

* Documentation updates.

* Implement oneof support (Issue #174). Protobuf 2.6.0 or newer is now
required to build protobuf-c.

* Print leading comments for enum, message, and field definitions into
generated header files (Issue #175).

Github does no speak http (only https), so mirror the distfile on
MASTER_SITE_LOCAL (suggested by mat@).

Rename DOCS option to DOXYGEN (suggested by pawel for net/axa).

Use PORTDOCS=* instead of listing DOXYGEN generated files in pkg-plist.

Use options helpers instead of .if (suggested by mat@).

Use INSTALL_TARGET=install-strip instead of ${STRIP_CMD}.

Test Plan

Test downloads from both MASTER_SITES.

Test build with DOXYGEN option both on and off.

Rebuild all dependent ports.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

truckman retitled this revision from to Upgrade devel/protobuf-c to version 1.1.0.
truckman updated this object.
truckman edited the test plan for this revision. (Show Details)
truckman added a reviewer: mat.
truckman set the repository for this revision to rP FreeBSD ports repository.

I really am not a fan of the move from DOCS to DOXYGEN, but if it makes people happy…

As a side note, it would be *great* if you could use arc diff --create (from the php5-arcanist package) to create your reviews, it would enable all the features of phabric, like adding context, which is a great thing, I would have been able to see what's between line 12 and 26 of the Makefile in the diff here instead of having to go and look at the actual Makefile 0:-)

Makefile
8 ↗(On Diff #3019)

LOCAL/truckman/farsight

46 ↗(On Diff #3019)

I think this should be:

cd ${WRKSRC}/html/ && ${COPYTREE_SHARE} . ${STAGEDIR}${DOCSDIR}/html

As it seems to be the only thing to be installed in the docs, maybe it does not need to be in the html subdirectory.

Makefile
25–26 ↗(On Diff #3019)

You should use ${PORTNAME} here, better yet would be to not define those variables and use ${PORTNAME} in MASTER_SITES directly.

Some of my ports have other docs in addition to the doxygen generated docs. For those I was calling the option, HTML_DOCS. The suggestion was made to change the latter to DOXYGEN as a warning to the user that building the port will drag in all the doxygen stuff. I think renaming the option here is good for consistency.

By the time FreeBSD mail started working and I got my account verification email for reviews, my port builder box was busy for the duration, so I don't have arcanist installed yet. I should be able to do that tonight.

Makefile
8 ↗(On Diff #3019)

I first saw that in a bunch of commit messages that flew by last night, but it doesn't seem to be documented anywhere.

Makefile
8 ↗(On Diff #3019)

Sure it is. https://www.freebsd.org/doc/en/books/porters-handbook/makefile-distfiles.html#makefile-master_sites

Mmmm, LOCAL is missing from the list, I don't understand why, I sync'ed it a few months back. It's mentionned in its old for in https://www.freebsd.org/doc/en/books/porters-handbook/slow-sources.html

I'll cook something up in there today :-)

Ah, other than that, when the distfile is from github, I usually put the LOCAL before the github site, as it's faster to respond, see https://svnweb.freebsd.org/ports/head/textproc/cdiff/Makefile?annotate=374183#l7 for instance.

Makefile
8 ↗(On Diff #3019)

Ah, no, the list in the PHB is not complete, as any MASTER_SITE_FOO can be used as FOO/xxx and xxx will be used as the subdir. Ok, I'll definitively cook up something.

Makefile
8 ↗(On Diff #3019)

When did the new syntax happen? I never noticed it until the last day
or so in some commit messages. It wasn't the the PHB the last
time I looked at that chapter (which wasn't that long ago).

I do like the new syntax :-)

I've got a couple of reasons for the ordering:

  • I prefer SSL downloads and only added the LOCAL mirror to accomodate sites that require downloads through cripped proxies (probably to allow the firewall to do content inspection), which I suspect is the reason that portlint squawks if there isn't an http url.
  • New distfiles will show up first on github. I don't know if portscout checks all MASTER_SITES, but if it only looks at the first, then it will nag me to copy the new distfile to LOCAL and update the port.
46 ↗(On Diff #3019)

Since there is no hierarchy under ${WRKSRC}/html, both give the same result and using INSTALL_DATA is less verbose. The

cd /some/path && ${COPYTREE_SHARE} ...

idiom has always seemed awkward to me.

rsync /some/path ...

or

rsync /some/path/ ...

does the right thing for me outside the world of ports, but I use it infrequently enough that I always have to look up which one to use.

I prefer to keep the extra html directory level for consistency and in case this port grows other documentation. For instance, it's fairly easy to tell doxygen to also generate PDF.

Makefile
8 ↗(On Diff #3019)

Mmmm, bit of digging, the new syntax happened in rP169110, which is in 2006, it was documented in the PHB in 2010 in rD36636, so, it must have been a long time since you looked at it :-p

The new syntax is great, when it works, and there is also a shorthand for some SUBDIRS, like CPAN:FOO which will give F/FO/FOO, that I don't think are documented, and now that I look at it, it's the only one.

Also, both your reasons are good for the ordering, so, do keep it :-)

46 ↗(On Diff #3019)

Well, rsync is great, rsync is not in the base system, and depending on it just to be able to install some files is way overkill.

cd && find | cpio always worked great for me, and I do use it often :-)

Makefile
8 ↗(On Diff #3019)

Ah, no, the new syntax is documented:

If the original tarball is part of one of the popular archives such as SourceForge,
GNU, or Perl CPAN, it may be possible refer to those sites in an easy compact form
using predefined macros (for example, SF, GNU or CPAN). Set MASTER_SITES to one of
these values. Here is an example:

MASTER_SITES=	GNU/make

The older expanded format can still be used, although there really is no reason to do so:

MASTER_SITES=		${MASTER_SITE_GNU}
MASTER_SITE_SUBDIR=	make
truckman updated this object.
truckman edited the test plan for this revision. (Show Details)
truckman edited edge metadata.

MASTER_SITES changes suggested by mat@.

Makefile
46 ↗(On Diff #3019)

Not saying that I'd want to use rsync here. I'd be happy if

${COPYTREE_SHARE} ${WRKSRC}/html ${STAGEDIR}${DOCSDIR}

created the html directory under ${DOCSDIR} and copied all the files over. That would be ideal.

Makefile
8 ↗(On Diff #3019)

... and the documentation for the old syntax is now pretty sparse. Maybe I was looking at Mk/bsd.sites.mk. I remember having to figure out the subdir stuff fairly recently, probably last year. Also the github stuff.

Makefile
8 ↗(On Diff #3019)

I try very hard to steer people away from reading Mk/*, I've done tremendous work on the porter's handbook in 2014 so that it was up-to-date. And as far as I know, every major thing is documented there, and there's no minor thing that's not in it too :-)

46 ↗(On Diff #3019)

well, to do that, you can do:

cd ${WRKSRC} && ${COPYTREE_SHARE} html ${STAGEDIR}${DOCSDIR}

:-)

It will replicate the whole directory structure you give it in the destination directory.

mat edited edge metadata.

Macro shipit:

(sorry, I love this image)

This revision is now accepted and ready to land.Jan 7 2015, 10:59 PM