Page MenuHomeFreeBSD

devel/chromium-gn: meta-build system that generates NinjaBuild files
ClosedPublic

Authored by luca.pizzamiglio_gmail.com on Jul 10 2017, 1:05 PM.

Details

Summary

This tool is currently part of Chromium and it's needed as
separate ports as an important component to build electron.io
Changes:

  1. adding a new port called chromium-gn in the devel category
  2. devel/chromium-gn is designed as slave port of chromium
  3. www/chromium has some changes in order to be considered as master
Test Plan

Build devel/chromium-gn and www/chromium on all currently supported FreeBSD versions

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mmokhi updated this revision to Diff 30632.Jul 10 2017, 10:21 PM
mmokhi edited edge metadata.

Fix some points to make it build in a clean poudriere jail:

  • Change portname in a more meaningful thing as a slaveport (Added a PKGNAME_SUFFIX) This actually will help us not redifining DISTNAME, And because of that make makesum wont break the master port's distinfo. This also helps the revision of the slaveport meaningfully being bumped with its master port.
  • Add TEST_DISTFILES to both master and slave ports
  • Fix LICENSE Error

I also tested the build/run the gn into poudriere jails and it worked fine :-)

mmokhi accepted this revision.Jul 10 2017, 10:23 PM
mmokhi added a subscriber: feld.

To me it seems fine.
I'll wait for @mat or @feld comments also.
Then hopefully I can commit it :-)

This revision is now accepted and ready to land.Jul 10 2017, 10:23 PM
mat edited edge metadata.Jul 11 2017, 12:19 PM

Also, chromium's maintainer must be consulted before this goes in.

www/chromium/Makefile
4 ↗(On Diff #30632)

Not really needed, the PORTNAME is never changed.

10 ↗(On Diff #30632)

Same here.

206 ↗(On Diff #30632)

Any reason for this ?

240 ↗(On Diff #30632)

This should end at the end of the target, and a new .if should be opened afterwards, to avoid brain melting.

mmokhi updated this revision to Diff 30644.Jul 11 2017, 1:17 PM
mmokhi edited edge metadata.

-Fix brainmelting part (complex/unclear ifs in Makefile)
-Revert a locally commented part

This revision now requires review to proceed.Jul 11 2017, 1:17 PM
mmokhi added inline comments.Jul 11 2017, 1:18 PM
www/chromium/Makefile
4 ↗(On Diff #30632)

But this is how we did like in mysql ports, -server as master and -client as slave.

mat added inline comments.Jul 11 2017, 9:53 PM
www/chromium/Makefile
4 ↗(On Diff #30632)

Well, the ?= was made obsolete in rP74356 with MySQL 4.0.9 14 years ago, and was probably forgotten and copied around ever since.

?= is used to set a variable unless it is already set. In this case, the variable is always chromium. It should even be removed from the slave port definition.

mmokhi added inline comments.Jul 11 2017, 10:06 PM
www/chromium/Makefile
4 ↗(On Diff #30632)

Ahhh.
Makes sense then.
Thanks for the point :)
Going to fix it!

mmokhi updated this revision to Diff 30652.Jul 11 2017, 10:07 PM

-Delete unneeded ?= symbols from chromium Makefile
-Delete redundant PORTNAME from slave port

mat added inline comments.Jul 11 2017, 10:11 PM
devel/Makefile
767 ↗(On Diff #30652)

as the package is called chromium-gn, the directory should too.

mmokhi updated this revision to Diff 30654.Jul 11 2017, 10:24 PM

-Rename devel/gn to devel/chromium-gn regarding to package name

mmokhi updated this revision to Diff 30655.Jul 11 2017, 10:25 PM

-Rename devel/gn to devel/chromium-gn regarding to package name

mat accepted this revision.Jul 12 2017, 12:40 PM

Looks ok to me. You still need to get approval from chromium's maintainer.

This revision is now accepted and ready to land.Jul 12 2017, 12:40 PM

Thanks a lot.
I'll add one of chromium@ people here too.

swills added a subscriber: swills.Jul 12 2017, 2:27 PM

I support the idea of a separate port for gn. It could use useful for several things, including a potential ports for dart and skia, and upstream is considering a standalone release given the number of current and potential non-chromium users. See https://groups.google.com/a/chromium.org/forum/#!topic/gn-dev/pAYVwEW2RsI for example. FWIW, we would only be the second to package it separately, see https://repology.org/metapackage/gn/packages for example.

Also, I think the deps would need to be adjusted more. I think it shouldn't have any lib or run depends, since looking at the gn binary itself, it doesn't have any dependencies outside of what's in base:

https://gist.github.com/swills/5994db99b3ac26a3dac3eab94ab7fc83

luca.pizzamiglio_gmail.com edited edge metadata.
  • Removing unuseful dependencies
  • Fixing the install target after PORTNAME renaming
This revision now requires review to proceed.Jul 14 2017, 3:49 PM
cpm added subscribers: andrew, cpm.Jul 18 2017, 4:37 PM

I'm a bit off for incoming changes because I'm testing chromium build on arm64 (thanks to @andrew) but at first sight it looks good to me. It's still building....

This can be speeded up by providing the poudriere build logs. Luca, would you mind providing us a poudriere build log for the supported releases?

Thanks for your work.

In D11554#241100, @cpm wrote:

I'm a bit off for incoming changes because I'm testing chromium build on arm64 (thanks to @andrew) but at first sight it looks good to me. It's still building....
This can be speeded up by providing the poudriere build logs. Luca, would you mind providing us a poudriere build log for the supported releases?
Thanks for your work.

Thanks for response/review.
Do you want poudriere logs for chromium? or gn?

cpm retitled this revision from devel/gn: meta-build system that generates NinjaBuild files to devel/chromium-gn: meta-build system that generates NinjaBuild files.Jul 18 2017, 4:47 PM
cpm edited the summary of this revision. (Show Details)
cpm edited the test plan for this revision. (Show Details)
cpm added a comment.Jul 18 2017, 4:50 PM
In D11554#241100, @cpm wrote:

I'm a bit off for incoming changes because I'm testing chromium build on arm64 (thanks to @andrew) but at first sight it looks good to me. It's still building....
This can be speeded up by providing the poudriere build logs. Luca, would you mind providing us a poudriere build log for the supported releases?
Thanks for your work.

Thanks for response/review.
Do you want poudriere logs for chromium? or gn?

At least for chromium but for both ports it would be much better.

cpm added inline comments.Jul 20 2017, 12:23 AM
www/chromium/Makefile
23 ↗(On Diff #30787)

Remove the backslash.

32 ↗(On Diff #30787)

Add the missing backslash.

Fixing backslashes.

Started the build of chromium, it will take some time :)

I've built chromium-ng and chromium on FreeBSD 11.0 and 10.3 on adm64.

Logs here: https://drive.google.com/open?id=0B_S1y98YTyESODlfNEh6UndWekE

cpm added a comment.Jul 21 2017, 12:09 PM

I've built chromium-ng and chromium on FreeBSD 11.0 and 10.3 on adm64.
Logs here: https://drive.google.com/open?id=0B_S1y98YTyESODlfNEh6UndWekE

Macro lgtm:

jbeich added a subscriber: jbeich.Jul 25 2017, 6:22 PM

... including a potential ports for dart and skia ...

Future versions of www/firefox may depend on gn as well.

www/chromium/Makefile
69 ↗(On Diff #30995)

How much arch-specific code gn contains? Would it fail to build on non-x86 e.g., armv6, aarch64, powerpc*, sparc64 or mips*?

91 ↗(On Diff #30995)

Is even USES=compiler:c++11-lib not enough for gn (unlike chromium)? Clang/libc++ aren't stable on big-endian architectures.

Relaxing dependencies even more, requiring c++11-libs and, probably, it works on more architecture.

build logs will follow

Here you can find the build log of chromium and chromium-ng on all supported FreeBSD versions.

https://drive.google.com/drive/folders/0B_S1y98YTyESODlfNEh6UndWekE?usp=sharing

it works on more architecture.

I confirm. The new port builds fine on 11.1 aarch64/armv6 or with lang/gcc6 (via bug 219275). lang/gcc5 (or older) fail due to header bootlegging.

This revision was automatically updated to reflect the committed changes.