upstreaming all the *BSD related patches to chromium

558 views
Skip to first unread message

Robert Nagy

unread,
May 14, 2019, 10:16:18 AM5/14/19
to Chromium-dev, dpr...@chromium.org
Hello everyone,

I am Robert from the OpenBSD project, the maintainer of the Chromium port for the past 9 years.

It is time that the *BSD related patches are upstreamed because it is getting
really hard to follow all the code moving and new features that is happening at
every stable release of chromium.


Almost all of the OpenBSD and FreeBSD patches are the same and there are also 
OpenBSD and FreeBSD specific parts, but that is the not a concern at this point.

I've created a CL as a first step to set the OS specific defines in the build system.

Now there is a problem. We do not know what would be the best way to go.
There are two options:

1) set is_bsd and OS_BSD at the specific is_linux and OS_LINUX parts
This will introduce a lot of is_linux || is_bsd and OS_LINUX || OS_BSD checks
to the system, but it is a more separated way of handing the platforms.

2) set is_linux and OS_LINUX on *BSD (like Chromeos) and exclude stuff when not used
This will introduce some is_linux && !is_bsd OS_LINUX && !OS_BSD checks
to the system.
This way me and the FreeBSD guys can always pick up new Linux specific code and look
into it right away, instead of searching around the huge git repos for Linux specific changes.
Chromium developers do not have to take care of BSD specific defines because we'll just
use the Linux specific code.

Obviously we cannot expect the thousands of Chromium developers to always take care of
the BSD defines themselves, so from my point of view we can indeed go for using 
is_linux and OS_LINUX by default for *BSD and then we can nuke the really linux specific
code out on *BSD by using OS_BSD/OS_OPENBSD/OS_FREEBSD depending on what we
are talking about.

I really want to upstream these changes now especially with all the other projects using
the chromium codebase. I don't want to maintain hundreds of local patches for all of them.

So just tell me which way to go please and I will redo the patches that way.

Dirk at the CL above suggested that I write a mail to this group so that we can reach
a consensus.

Thank you.

Dirk Pranke

unread,
May 17, 2019, 12:39:38 PM5/17/19
to Robert Nagy, Chromium-dev
Hi Robert,

Thanks for the message. I'm sorry no one replied on the thread yet, I think people are still considering things.

I think one question people have is how much stuff actually needs to be patched, which isn't clear (at least not to me yet).

You pointed us at http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/ , but that directory seems to have patches for a lot of files that no longer exist (e.g., GYP files).

Is there some way to see just the list of patches you actually apply, or one big patch? Ideally maybe something that could be uploaded to Gerrit?

-- Dirk 

Nico Weber

unread,
May 17, 2019, 12:46:04 PM5/17/19
to rober...@gmail.com, Chromium-dev, Dirk Pranke
I don't have an opinion on accepting bsd-specific patches, but below is how I'd answer your question about the is_bsd bit.

From: Robert Nagy <rober...@gmail.com>
Date: Tue, May 14, 2019 at 10:16 AM
To: Chromium-dev
Cc: <dpr...@chromium.org>

Hello everyone,

I am Robert from the OpenBSD project, the maintainer of the Chromium port for the past 9 years.

It is time that the *BSD related patches are upstreamed because it is getting
really hard to follow all the code moving and new features that is happening at
every stable release of chromium.


Almost all of the OpenBSD and FreeBSD patches are the same and there are also 
OpenBSD and FreeBSD specific parts, but that is the not a concern at this point.

I've created a CL as a first step to set the OS specific defines in the build system.

Now there is a problem. We do not know what would be the best way to go.
There are two options:

1) set is_bsd and OS_BSD at the specific is_linux and OS_LINUX parts
This will introduce a lot of is_linux || is_bsd and OS_LINUX || OS_BSD checks
to the system, but it is a more separated way of handing the platforms.

2) set is_linux and OS_LINUX on *BSD (like Chromeos) and exclude stuff when not used
This will introduce some is_linux && !is_bsd OS_LINUX && !OS_BSD checks
to the system.

For this question, I'd recommend option 1. We're not super happy with is_linux being set for Chrome OS and thought about changing it, so we probably don't want to emulate that.
 
This way me and the FreeBSD guys can always pick up new Linux specific code and look
into it right away, instead of searching around the huge git repos for Linux specific changes.
Chromium developers do not have to take care of BSD specific defines because we'll just
use the Linux specific code.

Obviously we cannot expect the thousands of Chromium developers to always take care of
the BSD defines themselves, so from my point of view we can indeed go for using 
is_linux and OS_LINUX by default for *BSD and then we can nuke the really linux specific
code out on *BSD by using OS_BSD/OS_OPENBSD/OS_FREEBSD depending on what we
are talking about.

Right, the expectation here should be that Chromium devs won't consider BSD when changing things.
 

I really want to upstream these changes now especially with all the other projects using
the chromium codebase. I don't want to maintain hundreds of local patches for all of them.

So just tell me which way to go please and I will redo the patches that way.

Dirk at the CL above suggested that I write a mail to this group so that we can reach
a consensus.

Thank you.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/0daf7549-7e96-4b87-a4a8-244fac2f4da3%40chromium.org.

Alexei Svitkine

unread,
May 17, 2019, 12:55:05 PM5/17/19
to Nico Weber, rober...@gmail.com, Chromium-dev, Dirk Pranke
I do worry about the code becoming a lot messier with almost every OS_LINUX ifdef also needing to have an OS_BSD.

Perhaps we can have some separate ifdef such as DESKTOP_POSIX? (Or some better name that makes it clear that Mac is not included?)

Nico Weber

unread,
May 17, 2019, 1:03:46 PM5/17/19
to Alexei Svitkine, rober...@gmail.com, Chromium-dev, Dirk Pranke
Looking through https://cs.chromium.org/search/?q=OS_LINUX&sq=package:chromium , I don't think it's "almost every". Lots of the OS_LINUX stuff is actually linux specific. 15% of these are actually OS_LINUX && !OS_CHROMEOS (https://cs.chromium.org/search/?q=OS_LINUX.*!defined.OS_CH&sq=package:chromium&type=cs), and I think with the BSDs this will likely be a good chunk higher.

Robert Nagy

unread,
May 17, 2019, 1:20:15 PM5/17/19
to Dirk Pranke, Chromium-dev
Hi Dirk!

The CVS repository also includes old files, but in the Attic and those are not used.
Anyway here is the currently used patches in one gigantisch patch:  http://nerd.hu/tmp/c1.diff

However those patches should not be used because it's a mix of using is_linux on BSD
but not OS_LINUX.

I've already started working on defining OS_LINUX as well, and the number of files needed
to be patched just in base/ were around the same as adding OS_BSD everywhere.

base/ contains most of the really linux specific code, except for Zygote so by to looks of
it, I think using OS_LINUX on *BSD will yield around 200 patches less than the other way
around.

Regards,
Robert

Robert Nagy

unread,
May 17, 2019, 1:22:19 PM5/17/19
to Alexei Svitkine, Nico Weber, Chromium-dev, Dirk Pranke
I agree. A lot of OS_LINUX visible code is actually POSIX, but might not to be used for MACOSX. But to be honest I would rather go through with the BSD patches
first and after that, I can help with the "posixification" of the code.

Robert Nagy

unread,
May 17, 2019, 1:25:22 PM5/17/19
to Dirk Pranke, Chromium-dev
Just to extend my previous reply.

Since there are some hacks that we currently use I am also cleaning up
some stuff in those patches while I am submitting them, so don't take everything
in there for granted.

Also uploading this big diff to one huge review would not be a good idea IMO, because
the speed of code changes all over the tree would make it impossible for me to keep
the whole thing up-to-date. I think per-directory is the way to go.

Robert Nagy

unread,
May 17, 2019, 1:32:10 PM5/17/19
to Dirk Pranke, Chromium-dev
Sorry I missed to include that http://nerd.hu/tmp/c2.diff is the diff against base
in the case of defining OS_LINUX/is_linux for a comparsion with the base
part of c1.diff.

Jochen Eisinger

unread,
May 21, 2019, 3:35:20 AM5/21/19
to rober...@gmail.com, Chromium-dev, Dirk Pranke
Hey Robert,

thanks for reaching out. We’ve discussed the question of whether or not to accept those patches upstream in the chromium repo amongst the eng review group.

We’re happy for others to make downstream derivatives of chromium, and we recognize the costs in doing so. However, we also want to maintain focus for the chromium project, and that means being choosy in which platforms we sign everybody up to support.

Therefore, we've decided to not accept patches introducing *BSD support at this time.

best
-jochen


Robert Nagy

unread,
May 21, 2019, 4:07:00 AM5/21/19
to Jochen Eisinger, Chromium-dev, Dirk Pranke
Hey Jochen,

I understand the decision, but then can you rationale the fact of having AIX ifdefs all around the chromium tree? :)

Thanks

Jochen Eisinger

unread,
May 21, 2019, 4:11:41 AM5/21/19
to Robert Nagy, Chromium-dev, Dirk Pranke
afaik the AIX support is for V8 which shares chromium's build infrastructure, and supports more platforms than chromium does.

Robert Nagy

unread,
May 21, 2019, 4:15:43 AM5/21/19
to Jochen Eisinger, Chromium-dev, Dirk Pranke
Ok, thanks for the clarification. 

Graham Perrin

unread,
Nov 30, 2021, 10:52:19 AM11/30/21
to Chromium-dev, Jochen Eisinger, Chromium-dev, Dirk Pranke, Robert Nagy
On Tuesday, 21 May 2019 at 08:35:20 UTC+1 Jochen Eisinger wrote:

… we've decided to not accept patches introducing *BSD support at this time. …

Re: the decision, please, might there be a review in the next year or so?


Thanks
Reply all
Reply to author
Forward
0 new messages