upstreaming all the *BSD related patches to chromium

[Robert Nagy]

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.

The current patches are available at: http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/

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.

https://chromium-review.googlesource.com/c/chromium/src/+/1608740

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]

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]

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.

The current patches are available at: http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/

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.

https://chromium-review.googlesource.com/c/chromium/src/+/1608740

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]

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?)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAMGbLiG0HFRd7bUiZCQg%2BOk_Z_RdT102fDtQqwkw1C7CqM0G0Q%40mail.gmail.com.

[Nico Weber]

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]

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]

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]

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]

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]

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]

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]

afaik the AIX support is for V8 which shares chromium's build infrastructure, and supports more platforms than chromium does.

[Robert Nagy]

Ok, thanks for the clarification. 

[Graham Perrin]

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?

I'm here via <https://github.com/freebsd/chromium#readme>.

Thanks