About naming of files/directories shared by Linux and Android

[Xianzhu Wang]

Hi,

Android and Linux share some source files, while not all Linux files are applicable on Android. Sometimes we want to reuse some files/directories that are originally for linux only. The files were named "_linux.(cc|h)" and directories named "linux". In current build/filename_rules.gypi, these files are by default excluded on Android.

To include them on Android, we may use several different methods:

1. Rename 'linux' to 'posix'. Need to exclude the files on mac, that is: posix - mac

2. Rename 'linux' to 'linuxish'. In build/filename_rules.gypi, linuxish = linux + bsd + android ~= posix - mac

3. Use a tricky 'target_condition' section to include the files without renaming.

   (In gyp, 'sources/' under normal 'condition' doesn't work because the rules in build/filename_rules.gypi have higher priority)

I think we should keep consistent. I'd like 2 but maybe we need a better name than 'linuxish'.

BTW, another issue about linux and android is that in WebKit OS(LINUX) includes OS(ANDROID), but in Chromium OS_LINUX doesn't include OS_ANDROID. I think we should also make them consistent. I suggest to change WebKit to conform to the Chromium rule. 

What are your opinions?

Xianzhu

[Adam Barth]

There is some discussion of this topic on this CL:

http://codereview.chromium.org/10544103/

Apparently, linuxish was introduced in
http://codereview.chromium.org/9838033. Adding Carlos, Mark, and
Peter in case they have thoughts on the origin of the term.

Adam

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev

[Jonathan Dixon]

(1) only makes sense if the platform dependency really is posix. There can be things that are common to (desktop) linux and andriod that don't correspond to any posix standard. "/proc" is one things that comes to mind (although really, that's not part of official android API either).

So I expect (at least) two solutions are needed depending on the specific scenario.

Maybe some concrete examples will help?

On 14 June 2012 19:39, Xianzhu Wang <wangx...@chromium.org> wrote:

Hi,

Android and Linux share some source files, while not all Linux files are applicable on Android. Sometimes we want to reuse some files/directories that are originally for linux only. The files were named "_linux.(cc|h)" and directories named "linux". In current build/filename_rules.gypi, these files are by default excluded on Android.

To include them on Android, we may use several different methods:

1. Rename 'linux' to 'posix'. Need to exclude the files on mac, that is: posix - mac

2. Rename 'linux' to 'linuxish'. In build/filename_rules.gypi, linuxish = linux + bsd + android ~= posix - mac

3. Use a tricky 'target_condition' section to include the files without renaming.

   (In gyp, 'sources/' under normal 'condition' doesn't work because the rules in build/filename_rules.gypi have higher priority)

This is a quick fix to introduce, but I personally dislike (3) as it gets confusing over the long term.

 

I think we should keep consistent. I'd like 2 but maybe we need a better name than 'linuxish'.

BTW, another issue about linux and android is that in WebKit OS(LINUX) includes OS(ANDROID), but in Chromium OS_LINUX doesn't include OS_ANDROID. I think we should also make them consistent. I suggest to change WebKit to conform to the Chromium rule. 

What are your opinions?

Xianzhu

--

[Adam Barth]

On Thu, Jun 14, 2012 at 8:50 PM, Jonathan Dixon <jo...@chromium.org> wrote:
> (1) only makes sense if the platform dependency really is posix. There can
> be things that are common to (desktop) linux and andriod that don't
> correspond to any posix standard. "/proc" is one things that comes to mind
> (although really, that's not part of official android API either).
>
> So I expect (at least) two solutions are needed depending on the specific
> scenario.
>
> Maybe some concrete examples will help?

This came up in the context of
<http://trac.webkit.org/changeset/120394>, which is about font
configuration in WebKit. In particular,
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/src/linuxish/WebFontRendering.cpp
has linuxish in its name because it's used by both chromium-android
and chromium-linux.

Adam

[John Abd-El-Malek]

I find linuxish a confusing and ambiguous name, and looking at the WebKit bug, it seems I'm not the only one.

How did this name get picked? Can you point me to when this got decided?

[Mark Mentovai]

Xianzhu Wang wrote:

Android and Linux share some source files, while not all Linux files are applicable on Android. Sometimes we want to reuse some files/directories that are originally for linux only. The files were named "_linux.(cc|h)" and directories named "linux". In current build/filename_rules.gypi, these files are by default excluded on Android.

To include them on Android, we may use several different methods:

1. Rename 'linux' to 'posix'. Need to exclude the files on mac, that is: posix - mac

This is definitely not correct.

2. Rename 'linux' to 'linuxish'. In build/filename_rules.gypi, linuxish = linux + bsd + android ~= posix - mac

This is only correct if the files in question are actually appropriate on the various BSDs and other systems people have ported Chromium to.

3. Use a tricky 'target_condition' section to include the files without renaming.

   (In gyp, 'sources/' under normal 'condition' doesn't work because the rules in build/filename_rules.gypi have higher priority)

If Android wants those _linux files, maybe it should switch to including them by default. It is, after all, Linux.

Which is smaller, the number of _linux files that Android would need to exclude if they were included by default, or the number of _linux files that it needs to include because they’re currently excluded by default?

When Android doesn’t want to use an existing _linux file, what is the reason? Are those files misnamed as it is? We have extensions for toolkits and other things as well, so would _x11 or _gtk or something else be more appropriate in those cases?

Unfortunately, everything in this area is always going to wind up a compromise.

[Jonathan Dixon]

On 15 June 2012 08:30, Mark Mentovai <ma...@chromium.org> wrote:

Xianzhu Wang wrote:

Android and Linux share some source files, while not all Linux files are applicable on Android. Sometimes we want to reuse some files/directories that are originally for linux only. The files were named "_linux.(cc|h)" and directories named "linux". In current build/filename_rules.gypi, these files are by default excluded on Android.

To include them on Android, we may use several different methods:

1. Rename 'linux' to 'posix'. Need to exclude the files on mac, that is: posix - mac

This is definitely not correct.

2. Rename 'linux' to 'linuxish'. In build/filename_rules.gypi, linuxish = linux + bsd + android ~= posix - mac

This is only correct if the files in question are actually appropriate on the various BSDs and other systems people have ported Chromium to.

3. Use a tricky 'target_condition' section to include the files without renaming.

   (In gyp, 'sources/' under normal 'condition' doesn't work because the rules in build/filename_rules.gypi have higher priority)

If Android wants those _linux files, maybe it should switch to including them by default. It is, after all, Linux.

This is also not correct. While Android is built on linux, the user library is different (bionic) and a lot of linux functionality is not guaranteed to be there on all devices and versions (e.g. /proc as I mentioned).

Put another way - it is possible for an OEM to implement a fully SDK/NDK compliant Android on a non-linux base.

Also on a practical side, we found that having OS_ANDROID imply OS_LINUX resulted in more special-casing to remove unwanted files than the reverse.

 

Which is smaller, the number of _linux files that Android would need to exclude if they were included by default, or the number of _linux files that it needs to include because they’re currently excluded by default?

When Android doesn’t want to use an existing _linux file, what is the reason? Are those files misnamed as it is? We have extensions for toolkits and other things as well, so would _x11 or _gtk or something else be more appropriate in those cases?

Unfortunately, everything in this area is always going to wind up a compromise.

--

[Mark Mentovai]

Jonathan Dixon wrote:

Also on a practical side, we found that having OS_ANDROID imply OS_LINUX resulted in more special-casing to remove unwanted files than the reverse.

Then it sounds like the current state is already about as good as it’s going to get?

Sorry, I know it’s not satisfying, but as you can see, everything really is a compromise.

[Brett Wilson]

I agree. If Android wants Linux files, you can just add them to the
project section explicitly. OS_LINUX || OS_ANDROID seems fine as a
preprocessor define approach.

Brett

[Brett Wilson]

On Fri, Jun 15, 2012 at 8:11 AM, John Abd-El-Malek <j...@chromium.org> wrote:
> I find linuxish a confusing and ambiguous name, and looking at the WebKit
> bug, it seems I'm not the only one.
>
> How did this name get picked? Can you point me to when this got decided?

I also don't like Linuxish.

Brett

[Mark Mentovai]

Brett Wilson wrote:

You’re free to choose a new name if there’s consensus. Carlos V. chose _linuxish while upstreaming things for Android Breakpad and I don’t personally have any problem with it, although he didn’t like the name either. The other suggestion was _lindroid, which I don’t like at all. As Adam pointed out, the code review was http://codereview.chromium.org/9838033.

[John Abd-El-Malek]

Do we really need a new name at all? i.e. option 3 would add the linux files that are necessary in a target_condition section. That seems the most reasonable to me given that Android port doesn't want all Linux files by default.

[Mark Mentovai]

John Abd-El-Malek wrote:

Do we really need a new name at all? i.e. option 3 would add the linux files that are necessary in a target_condition section. That seems the most reasonable to me given that Android port doesn't want all Linux files by default.

I guess not. There are only two _linuxish classes right now, they can just be renamed to _linux and the other platforms that want them can opt in.

[Xianzhu Wang]

Thanks all for your replies.

I'll summarize the existing files for:

1) how many "_linux" files need or don't need to be shared by Linux and Android; 

2) how many "_linux" files can be renamed to be toolkit (x11, gtk etc.) or library (fontconfig, freetype, etc.) specific.

I guess many of the "_linux" files belongs to 2). For each of them, whether it is shared by Android and Linux depends on whether the toolkit/library is available and should be used on Android.

I'm thinking of whether we should name a file based on whether the file is valid on the os/toolkit/library, or based on whether it is actually used. I prefer the latter. For example, for the files named with "_posix" but excluded on mac, some of them might be incorrect, but if the file can be compiled on mac but mac doesn't use it because mac has better native api, it seems we can still name it with "_posix". If a file is valid on a os/toolkit/library (e.g. posix) but not used in a specific case (e.g. mac) for some reason, it can be excluded in the specific gyp file.

Thanks,

Xianzhu

[Xianzhu Wang]

Some statistics in current Chromium codebase:

Number of shared:

  1. Renamed to 'posix':

  Found in git log of chromium repository, 16 files have been renamed from "*_linux*.cc" to "*_posix*.cc". More than half are directly related to Android.

  2. Renamed to 'linuxish':

  Only 3 cc files which are related to breakpad.

  3. Use 'target_condition' section to include the files without renaming:

  Only 4 under net/ and base/

Number of not shared:

  There are other 94 .cc files named with "_linux" and 56 files under directory 'linux' not shared by Android.

For the particular case of http://codereview.chromium.org/10544103/, my current opinion is to rename the '_linux.cc' files to '_harfbuzz.cc' because the underlying implementation in WebKit is under a directory named 'harfbuzz'.

Thanks,

Xianzhu

[Mark Mentovai]

Xianzhu Wang wrote:

For the particular case of http://codereview.chromium.org/10544103/, my current opinion is to rename the '_linux.cc' files to '_harfbuzz.cc' because the underlying implementation in WebKit is under a directory named 'harfbuzz'.

That file doesn’t seem to have anything about harfbuzz in it at all, so that sounds like a poor choice.

Given how we normally do things, the traditional name for this file would be _x11, and you’d have to opt it in on Android.

[Xianzhu Wang]

The file also doesn't seem to have anything about x11. And I don't think the situation is better for _x11 than _linux. Both of them don't cover Android and are excluded by default rules which require a tricky target_condition to override.

I thought of naming it harfbuzz because the following dependency chain (but now I also feel not good):

  render_view_harfbuzz.cc

-> third_party/WebKit/Source/WebKit/chromium/public/harfbuzz/WebFontRendering.h (to be done in https://bugs.webkit.org/show_bug.cgi?id=89228)

-> third_party/WebKit/Source/WebKit/chromium/src/harfbuzz/WebFontRendering.cpp (to be done in https://bugs.webkit.org/show_bug.cgi?id=89228)

-> third_party/WebKit/Source/WebCore/platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp

Just thought of another solution:

1. Move third_party/WebKit/Source/WebCore/platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp to third_party/WebKit/Source/WebCore/platform/graphics/skia/FontPlatformDataSkia.cpp. Actually the file only depends on Skia. Though the related features may depend on some libraries used by Skia, the interfaces are available in Skia on all platforms.

2. Move WebFontRendering.h and cpp out of subdirectory to make them available for all platforms.

3. Combine render_view_linux.cc into render_view_impl.cc.

4. We'll still have different font rendering configurations for different platforms. For Android, they will be in ui/gfx/font_render_params_android.cc.

Does this make sense?

Thanks,

Xianzhu

[Mark Mentovai]

Xianzhu Wang wrote:

Just thought of another solution:

1. Move third_party/WebKit/Source/WebCore/platform/graphics/harfbuzz/FontPlatformDataHarfBuzz.cpp to third_party/WebKit/Source/WebCore/platform/graphics/skia/FontPlatformDataSkia.cpp. Actually the file only depends on Skia. Though the related features may depend on some libraries used by Skia, the interfaces are available in Skia on all platforms.

2. Move WebFontRendering.h and cpp out of subdirectory to make them available for all platforms.

3. Combine render_view_linux.cc into render_view_impl.cc.

4. We'll still have different font rendering configurations for different platforms. For Android, they will be in ui/gfx/font_render_params_android.cc.

Does this make sense?

I think that at this point, since you’re working within the confines of the system as it already exists, you’re best off floating this idea on your code review.

[John Abd-El-Malek]

I assume you're talking about new files.

What about the existing "linuxish" files in chrome/webkit? Can we rename them to use linux and use the target_conditions to include them on Android?

[Mark Mentovai]

John Abd-El-Malek wrote:

What about the existing "linuxish" files in chrome/webkit? Can we rename them to use linux and use the target_conditions to include them on Android?

Sure, I agreed to that a couple of messages ago. I don’t think it’s controversial.

[John Abd-El-Malek]

cool. Android folks: will someone take care of this?

[Xianzhu Wang]

I'll do it.