936297 - expose posix_spawn_file_actions_t via Constants.libc

Status: RESOLVED FIXED

(Toolkit Graveyard :: OS.File, enhancement)

Description

[Myk Melez [:myk] [@mykmelez]]

Per bug 928168, comment 31, OSFileConstants.cpp should define a constant representing the size of posix_spawn_file_actions_t, so JavaScript code that uses js-ctypes to access libc functions like posix_spawn can declare that type with the appropriate size for the OS.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

That's pretty simple to do. If anybody wants to pick this bug, I can mentor it.

Comment 2

[Peiyong Lin[:lpy](UTC+8)]

I am willing to fix it, please assign this bug to me. Thanks.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

With pleasure.
Lin, if you need any assistance to get started with this bug, don't hesitate to come and chat with us at irc://irc.mozilla.org/#introduction or equivalently http://client00.chat.mibbit.com/?server=irc.mozilla.org&channel=%23introduction&nick=PelyongLin

Comment 4

[Peiyong Lin[:lpy](UTC+8)]

Attachment: bug936297.patch

After the compile error, I add '#include <spawn.h>', but I don't know whether it is necessary or not.

Comment 5

[David Teller [:Yoric] - still alive but not very active]

Comment on attachment 831261 [details] [diff] [review]
bug936297.patch

Review of attachment 831261 [details] [diff] [review]:
-----------------------------------------------------------------

r=me if you remove the unnecessary includes.

::: dom/system/OSFileConstants.cpp
@@ +17,5 @@
>  #endif // defined(XP_UNIX)
>  
>  #if defined(XP_LINUX)
>  #include <linux/fadvise.h>
> +#include <spawn.h>

That shouldn't be necessary, XP_LINUX is just a case of XP_UNIX.

@@ +22,5 @@
>  #endif // defined(XP_LINUX)
>  
>  #if defined(XP_MACOSX)
>  #include "copyfile.h"
> +#include <spawn.h>

Likewise for XP_MACOSX.

Comment 6

[Peiyong Lin[:lpy](UTC+8)]

Attachment: bug936297.patch

remove unnecessary include

Comment 7

[Ryan VanderMeulen [:RyanVM]]

This doesn't build on Android/B2G.
https://tbpl.mozilla.org/php/getParsedLog.php?id=30527734&tree=Try

Comment 8

[Peiyong Lin[:lpy](UTC+8)]

Attachment: bug936297.patch

Comment 9

[Myk Melez [:myk] [@mykmelez]]

lpy: when RyanVM said it doesn't build on B2G, I think he meant only that it doesn't build on the *Gonk* builds of B2G, as it should build fine on the *desktop* builds of B2G.

You could check if !defined(MOZ_WIDGET_GONK) to exclude Gonk builds, but that shouldn't be necessary, because ANDROID should be defined for Gonk builds, so building if !defined(ANDROID) should already exclude them.

Thus checking if !defined(ANDROID) should be sufficient to exclude both Fennec builds and Gonk builds of B2G.  Can you update the patch with that change?

Comment 10

[Myk Melez [:myk] [@mykmelez]]

From configure.in:

  if test -n "$gonkdir" ; then
  …
      AC_DEFINE(ANDROID)

  - https://github.com/mozilla/mozilla-central/blob/82551c3bfd07193ed63938c31978460ed59287dc/configure.in#L183-L272

Comment 11

[Peiyong Lin[:lpy](UTC+8)]

Thank you myk, I will update my patch soon.

Comment 12

[Peiyong Lin[:lpy](UTC+8)]

Attachment: bug936297.patch

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/14f9233f4b79

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/14f9233f4b79