Created attachment 242784 [details] Patch for brotli Master branch contains various fixes and some performance enhancements [1], unfortunately it doesn't seem to be given much time by Google so releases are lagging and maintainers have little time. While at it hook up "make test". 1: https://github.com/google/brotli/commit/1e61e972fbbfe59a03e643e444aeb2904bfe20bf Compile and runtime tested on FreeBSD 13.2-RELEASE (amd64) (make, make check-plist, make test) Poudriere testport OK 12.4-RELEASE (amd64) Poudriere testport OK 13.2-RELEASE (amd64) Tested with following users in Poudriere (13.2-RELEASE, amd64): www/cpp-httplib x11/xpra-html5 archivers/php-brotli archivers/py-brotlicffi databases/arrow databases/redisdesktopmanager devel/android-tools devel/apitrace devel/libsoup3 devel/qt6-base devel/woff2 graphics/brunsli graphics/libjxl net/wireshark print/fontforge print/freetype2 sysutils/nix (fails, marked broken in tree) www/c-icap www/elinks www/envoy (fails, marked broken in tree) www/nginx-full www/node16 www/node20 www/privoxy www/trafficserver www/wget2 x11/xpra x11-fonts/ots
Add @danfe as the www/envoy port maintainer.
(In reply to Daniel Engberg from comment #0) Hi Daniel, thanks for the patch. I see the static libraries have been removed from the package list. Does that an expected behavior?
(In reply to Sergey A. Osokin from comment #2) Hi, Yes, there's no way of building both libraries unless you modify the CMake file or do the build twice. Best regards, Daniel
As for the version update, maybe we could add some noise to https://github.com/google/brotli/issues/1014? Unfortunately this is Google, and it typically gives little shit about community's needs. :-( About the patch itself, you can probably drop DISTVERSIONPREFIX since you point at the commit, not a tag. Why does GH_TAGNAME include eight characters while we typically use seven (as long as it's sufficient)? I would've also dropped compiler:c++0x now that default compiler in all supported FreeBSD branches/architectures can into and defaults to C++11. DISTVERSION is misused here because there is no distfile or tag it could refer to, ergo it should be really spelled as PORTVERSION in this particular case. I'm fine with dropping static libraries, but less fine about switching from autotools to CMake, a heavier dependency pulling lots of stuff for itself, contrary to autotools which need nothing outside of the base system. But then again, this is not the hill I wonna die on, and ultimately a maintainer's call.
Created attachment 242812 [details] Patch for brotli v2 Drop DISTVERSIONPREFIX, USES=compiler:c++0x Use 7 chars in GH_TAGNAME
(In reply to Alexey Dokuchaev from comment #4) You're correct about DISTVERSIONPREFIX thanks for catching that one. Porters Handbook refers to DISTVERSION in all variants of USE_GITHUB and that's also cosistent in our tree so I don't see any reason to change that. I agree that we can drop USES=compiler in this case, especially since it's not defined within the project. 7 vs 8 chars are both used in examples in Porters Handbook but I've adjusted it to 7. In this case it pulls in a bunch of GNU tools so it does require a bunch of deps, makes our port less hacky and unit tests works.
(In reply to Daniel Engberg from comment #3) I believe it's better to keep static libraries in place.
Created attachment 242842 [details] Patch for brotli v3 Includes a simply patch to generic static libraries
Created attachment 242843 [details] Patch for brotli v4 Drop USES= cmake:noninja argument , I only used it for testing and never intended to submitting it.
Thanks for the update, Daniel. I have a concern about the API/ABI compatibility in case we move ahead with such updates. How is our strategy may look like? Do we need to bump a library number and if yes, then when? Another option we may have for the case is to create a separated port, i.e.: archivers/brotli-devel Please let me know your thoughts.
(In reply to Sergey A. Osokin from comment #10) > I have a concern about the API/ABI compatibility in case we move ahead with > such updates. Good point. Perhaps we can diff the current GitHub master with v1.0.9 and pull only useful/non-API-breaking commits? I'm also a bit worried about introducing new PKGVERSION with embedded date as a (very large) minor/patch version component which would cause us trouble if upstream all of sudden decides to release 1.0.9.1 because of some serious security bug. In other words, don't invent/introduce version numbers parallel to upstream ones as they might clash, e.g. with upstream or other distribution who makes the same mistake so now there are two distfiles with the same $name-$version but different checksums because they were packaged slightly differently.
(In reply to Alexey Dokuchaev from comment #11) > Good point. Perhaps we can diff the current GitHub master with v1.0.9 and pull > only useful/non-API-breaking commits? That sounds good to me. > I'm also a bit worried about introducing new PKGVERSION with embedded date as a > (very large) minor/patch version component which would cause us trouble if > upstream all of sudden decides to release 1.0.9.1 because of some serious > security bug. In other words, don't invent/introduce version numbers parallel > to upstream ones as they might clash, e.g. with upstream or other distribution > who makes the same mistake so now there are two distfiles with the same > $name-$version but different checksums because they were packaged slightly > differently. Well, we can keep PORTVERSION as is and since we have GH_TAGNAME we always know an introduced date of the commit, so we can just bump PORTREVISION (another idea is to keep a date in PORTVERSION, but that's a bit overkill at least to me :-).
(In reply to Sergey A. Osokin from comment #12) > Well, we can keep PORTVERSION as is and since we have GH_TAGNAME we always > know an introduced date of the commit, so we can just bump PORTREVISION Yes, this is one way to track some project's development without introducing/inventing new versions, and it works quite well. (In reply to Sergey A. Osokin from comment #10) > Another option we may have for the case is to create a separated port, i.e. > archivers/brotli-devel The problem with -devel ports is that two ports would conflict, so you can only have one of them installed at a time. This is fine for leaf ports (software facing end-users), but less so for dependencies (libraries).
I strongly disagree against knowingly falsely advertising a version number, that's just bad practise. Historically the project has never stepped away from X.X.X versioning and while that doesn't necessarily mean they never will I'd say it's very low probablility that they'll do. I can't find any API breaking and there isn't a huge list of changes but rather a bunch of fixes and improvments. https://github.com/google/brotli/compare/v1.0.9...master
(In reply to Daniel Engberg from comment #14) > I strongly disagree against knowingly falsely advertising a version number, > that's just bad practise. There's nothing wrong with slapping several patches on top of existing release, we and other distributions do that all the time. Now, creating new version numbers like 1.0.9.20230201 does look pretty bad and just asks for trouble. > [there's] low probablility that they'll do [release a version that would clash > with that]. Why gambling when we can simply avoid this situation in the first place? > I can't find any API breaking and there isn't a huge list of changes but > rather a bunch of fixes and improvements. Fixes and improvements can easily go under existing version 1.0.9 with port revision bump.
Except that it's ~60 commits which I would argue is a bit more than "several". Anyhow, looks like we might get a new version eventually. https://github.com/google/brotli/issues/1019
Created attachment 244269 [details] Patch for brotli v5 Update to 1.1.0rc
Created attachment 244558 [details] [PATCH] archivers/brotli: update to 1.1.0
Might also want to pull in https://github.com/google/brotli/pull/1070 Not sure if we need the hack for static libs (untested btw) in the end?
less than 50 ports depend on brotli so we don't do an exp-run for it
(In reply to Daniel Engberg from comment #19) Hi Daniel. What's wrong with the static libraries? Is there any reason we should not to ship those with the package? Thank you.
Hi, There are bunch of -DBROTLI*_SHARED_COMPILATION flags defined but it seems like they get passed correctly. Best regards, Daniel
Created attachment 244714 [details] [PATCH] archivers/brotli: update to 1.1.0, ver.3
(In reply to Daniel Engberg from comment #19) The PR has been added to the upstream, so I've updated our changes with those, attached.
Created attachment 244726 [details] Patch for brotli v6 * Backport upstream commit 741610efd335a8b6ff9be4c9bed643e0a74fdb6a * Make STATIC files option as it diverges from upstream Reference: https://github.com/google/brotli/commit/
Hi Sergey, I've attached a patch fixing missing man pages and missing patch for static libraries which your last patched was missing. I think this should be good to go unless you have any further input. Best regards, Daniel
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=62740dc1077c3c403e74ec6cae3a8b437ddb57d6 commit 62740dc1077c3c403e74ec6cae3a8b437ddb57d6 Author: Sergey A. Osokin <osa@FreeBSD.org> AuthorDate: 2023-09-09 19:40:56 +0000 Commit: Sergey A. Osokin <osa@FreeBSD.org> CommitDate: 2023-09-09 19:40:56 +0000 archivers/brotli: update from 1.0.9 to 1.1.0 (+) PR: 272005 Reviewed by: diizzy Tested by: diizzy archivers/brotli/Makefile | 24 +++++++------ archivers/brotli/distinfo | 8 +++-- .../brotli/files/extra-patch-static-libs (new) | 40 ++++++++++++++++++++++ archivers/brotli/pkg-plist | 19 ++++++---- 4 files changed, 71 insertions(+), 20 deletions(-)