Re: Should cheddar-generated headers be checked in?

2017-02-27 Thread Kartikaya Gupta 
On Mon, Feb 27, 2017 at 1:57 PM, Henri Sivonen  wrote:
> Now that the build.rs in commented out in the crates.io crate and the
> generated header is shipped in the crates.io crate:
> Considering that editing the vendored crates is not allowed, so I
> can't put moz.build files on the path to the headers, what's the
> appropriate way to make the m-c build system pick up headers from
> third_party/rust/encoding_c/include?
>

I haven't tried this myself, but you should be able to reference the
include from any moz.build file. e.g.

# in foo/bar/moz.build
EXPORTS += [
  ...
  "../../third_party/rust/encoding_c/include/MyHeader.h"
 ...
]

Does that not work?

Cheers,
kats
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-27 Thread Henri Sivonen 
On Mon, Feb 27, 2017 at 5:57 PM, Henri Sivonen  wrote:
> Maybe having the header generation as part of build.rs is more trouble
> than it's worth in the first place...

Now that the build.rs in commented out in the crates.io crate and the
generated header is shipped in the crates.io crate:
Considering that editing the vendored crates is not allowed, so I
can't put moz.build files on the path to the headers, what's the
appropriate way to make the m-c build system pick up headers from
third_party/rust/encoding_c/include?

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-27 Thread Henri Sivonen 
On Mon, Feb 27, 2017 at 2:38 PM, Henri Sivonen  wrote:
> Is there some way to disable build.rs for a vendored crate (without
> making cargo unhappy about the hashes)? (If the header is checked in,
> compiling cheddar in order to generate it at build time is useless.)

This seems not only relevant for build time but relevant to what
dependencies we vendor.

Right now, running ./mach vendor rust with gkrust depending on
https://crates.io/crates/encoding_c pulls in the dependencies for
cheddar, which we apparently don't already have in the tree.

Maybe having the header generation as part of build.rs is more trouble
than it's worth in the first place...

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-27 Thread Henri Sivonen 
On Mon, Feb 27, 2017 at 1:40 AM, Xidorn Quan  wrote:
>> When doing a parallel build, I see an interleave of C++ and Rust build
>> system output. What guarantees that a build.rs that exports headers
>> runs before the C++ compiler wants to see the headers?
>
> Oh, it's about C++ header generated from Rust?

Yes. cheddar, not bindgen.

> I don't think it is
> possible given the current architecture.
...
> So if it is Rust library exposing C API, it is probably responsibility
> of the Rust library to also provide a C header for other code to use.

I take it that I should check in the header, then.

Is there some way to disable build.rs for a vendored crate (without
making cargo unhappy about the hashes)? (If the header is checked in,
compiling cheddar in order to generate it at build time is useless.)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-26 Thread Xidorn Quan 
On Mon, Feb 27, 2017, at 02:33 AM, Henri Sivonen wrote:
> On Thu, Feb 23, 2017 at 4:37 PM, Ted Mielczarek 
> wrote:
> > On Thu, Feb 23, 2017, at 06:40 AM, Emilio Cobos Álvarez wrote:
> >> On Thu, Feb 23, 2017 at 08:25:30AM +0200, Henri Sivonen wrote:
> >> > On Wed, Feb 22, 2017 at 5:49 PM, Ted Mielczarek  
> >> > wrote:
> >> > > Given that
> >> > > the C API here is under your complete control, it seems like it's
> >> > > possible to generate a cross-platform header
> >> >
> >> > I believe the header is cross-platform, yes.
> >> >
> >> > > Alternately you could just generate it at build time, and we could pass
> >> > > the path to $(DIST)/include in a special environment variable so you
> >> > > could put the header in the right place.
> >> >
> >> > So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
> >> > naming conventions for the special variable? (I'm inferring from the
> >> > way you said it that DIST itself isn't being passed to the build.rs
> >> > process. Right?)
> >>
> >> FWIW, in Stylo we use MOZ_DIST[1], which is passed to the build script,
> >> not sure if it's stylo only though.
> >>
> >> [1]:
> >> https://searchfox.org/mozilla-central/rev/b1044cf7c2000c3e75e8181e893236a940c8b6d2/servo/components/style/build_gecko.rs#48
> >
> > So if you're only concerned about it working in Gecko--there you go!
> 
> Thanks. I'm interested both in the Gecko case and the general case.
> 
> When doing a parallel build, I see an interleave of C++ and Rust build
> system output. What guarantees that a build.rs that exports headers
> runs before the C++ compiler wants to see the headers?

Oh, it's about C++ header generated from Rust? I don't think it is
possible given the current architecture.

How build-time bindgen currently works is that, the Rust build script
generates binding files from exported C++ headers, and there is a test
to ensure their signatures are identical in the Rust side.

So if it is Rust library exposing C API, it is probably responsibility
of the Rust library to also provide a C header for other code to use.

- Xidorn
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-26 Thread Henri Sivonen 
On Thu, Feb 23, 2017 at 4:37 PM, Ted Mielczarek  wrote:
> On Thu, Feb 23, 2017, at 06:40 AM, Emilio Cobos Álvarez wrote:
>> On Thu, Feb 23, 2017 at 08:25:30AM +0200, Henri Sivonen wrote:
>> > On Wed, Feb 22, 2017 at 5:49 PM, Ted Mielczarek  
>> > wrote:
>> > > Given that
>> > > the C API here is under your complete control, it seems like it's
>> > > possible to generate a cross-platform header
>> >
>> > I believe the header is cross-platform, yes.
>> >
>> > > Alternately you could just generate it at build time, and we could pass
>> > > the path to $(DIST)/include in a special environment variable so you
>> > > could put the header in the right place.
>> >
>> > So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
>> > naming conventions for the special variable? (I'm inferring from the
>> > way you said it that DIST itself isn't being passed to the build.rs
>> > process. Right?)
>>
>> FWIW, in Stylo we use MOZ_DIST[1], which is passed to the build script,
>> not sure if it's stylo only though.
>>
>> [1]:
>> https://searchfox.org/mozilla-central/rev/b1044cf7c2000c3e75e8181e893236a940c8b6d2/servo/components/style/build_gecko.rs#48
>
> So if you're only concerned about it working in Gecko--there you go!

Thanks. I'm interested both in the Gecko case and the general case.

When doing a parallel build, I see an interleave of C++ and Rust build
system output. What guarantees that a build.rs that exports headers
runs before the C++ compiler wants to see the headers?

> I'm
> not aware that there's any better convention for this in Rust in the
> general sense.

:-(

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-23 Thread Emilio Cobos Álvarez 
On Thu, Feb 23, 2017 at 08:25:30AM +0200, Henri Sivonen wrote:
> On Wed, Feb 22, 2017 at 5:49 PM, Ted Mielczarek  wrote:
> > Given that
> > the C API here is under your complete control, it seems like it's
> > possible to generate a cross-platform header
> 
> I believe the header is cross-platform, yes.
> 
> > Alternately you could just generate it at build time, and we could pass
> > the path to $(DIST)/include in a special environment variable so you
> > could put the header in the right place.
> 
> So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
> naming conventions for the special variable? (I'm inferring from the
> way you said it that DIST itself isn't being passed to the build.rs
> process. Right?)

FWIW, in Stylo we use MOZ_DIST[1], which is passed to the build script,
not sure if it's stylo only though.

[1]: 
https://searchfox.org/mozilla-central/rev/b1044cf7c2000c3e75e8181e893236a940c8b6d2/servo/components/style/build_gecko.rs#48

> 
> -- 
> Henri Sivonen
> hsivo...@hsivonen.fi
> https://hsivonen.fi/
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform


signature.asc
Description: PGP signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-23 Thread Ted Mielczarek 
On Thu, Feb 23, 2017, at 06:40 AM, Emilio Cobos Álvarez wrote:
> On Thu, Feb 23, 2017 at 08:25:30AM +0200, Henri Sivonen wrote:
> > On Wed, Feb 22, 2017 at 5:49 PM, Ted Mielczarek  wrote:
> > > Given that
> > > the C API here is under your complete control, it seems like it's
> > > possible to generate a cross-platform header
> > 
> > I believe the header is cross-platform, yes.
> > 
> > > Alternately you could just generate it at build time, and we could pass
> > > the path to $(DIST)/include in a special environment variable so you
> > > could put the header in the right place.
> > 
> > So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
> > naming conventions for the special variable? (I'm inferring from the
> > way you said it that DIST itself isn't being passed to the build.rs
> > process. Right?)
> 
> FWIW, in Stylo we use MOZ_DIST[1], which is passed to the build script,
> not sure if it's stylo only though.
> 
> [1]:
> https://searchfox.org/mozilla-central/rev/b1044cf7c2000c3e75e8181e893236a940c8b6d2/servo/components/style/build_gecko.rs#48

So if you're only concerned about it working in Gecko--there you go! I'm
not aware that there's any better convention for this in Rust in the
general sense. Working with C code in projects built by Cargo seems to
be still fairly new territory. There are plenty of crates that build C
libraries and expose their API, and there are plenty of crates that
expose a C API from Rust code (many of which I'm sure use rusty-cheddar
to generate a header file), but I'm not sure there are strong
conventions for "this crate builds C code that relies on C code from
another crate".

-Ted
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-23 Thread Ted Mielczarek 
On Thu, Feb 23, 2017, at 08:27 AM, Nathan Froyd wrote:
> On Thu, Feb 23, 2017 at 1:25 AM, Henri Sivonen 
> wrote:
> >> Alternately you could just generate it at build time, and we could pass
> >> the path to $(DIST)/include in a special environment variable so you
> >> could put the header in the right place.
> >
> > So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
> > naming conventions for the special variable? (I'm inferring from the
> > way you said it that DIST itself isn't being passed to the build.rs
> > process. Right?)
> 
> We already pass MOZ_DIST as $(DIST)/include, fwiw:
> 
> http://dxr.mozilla.org/mozilla-central/source/config/rules.mk#941

n.b.: that shows us passing `MOZ_DIST=$(ABS_DIST)`, so you could use
`MOZ_DIST/include` in a Cargo build script.

-Ted
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-23 Thread Nathan Froyd 
On Thu, Feb 23, 2017 at 1:25 AM, Henri Sivonen  wrote:
>> Alternately you could just generate it at build time, and we could pass
>> the path to $(DIST)/include in a special environment variable so you
>> could put the header in the right place.
>
> So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
> naming conventions for the special variable? (I'm inferring from the
> way you said it that DIST itself isn't being passed to the build.rs
> process. Right?)

We already pass MOZ_DIST as $(DIST)/include, fwiw:

http://dxr.mozilla.org/mozilla-central/source/config/rules.mk#941

-Nathan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-22 Thread Henri Sivonen 
On Wed, Feb 22, 2017 at 5:49 PM, Ted Mielczarek  wrote:
> Given that
> the C API here is under your complete control, it seems like it's
> possible to generate a cross-platform header

I believe the header is cross-platform, yes.

> Alternately you could just generate it at build time, and we could pass
> the path to $(DIST)/include in a special environment variable so you
> could put the header in the right place.

So just https://doc.rust-lang.org/std/env/fn.var.html in build.rs? Any
naming conventions for the special variable? (I'm inferring from the
way you said it that DIST itself isn't being passed to the build.rs
process. Right?)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-22 Thread Ralph Giles 
I agree with Ted and Jack. For the specific case of the mp4parse
cheddar header, we currently check that in because it wasn't much
hassle, and cheddar pulled in syntex-syntax as a dependency which
added significantly to the build time. Eventually I want to use
runtime binding generation, for the usual reason that having generated
source code under version control is confusing. Porting cheddar to the
new `syn` crate would address the issue with build time.

FWIW,
 -r
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-22 Thread Jack Moffitt 
I think running bindgen at build time will be the best practice. That
is certainly what stylo and the spidermonkey bindings are aiming for.
This capability of bindgen is pretty new, so it will take a while
before the Rust community adapts.

jack.

On Wed, Feb 22, 2017 at 8:49 AM, Ted Mielczarek  wrote:
> On Wed, Feb 22, 2017, at 07:11 AM, Henri Sivonen wrote:
>> Looking at mp4parse, the C header is generated:
>> https://searchfox.org/mozilla-central/source/media/libstagefright/binding/mp4parse_capi/build.rs
>> But also checked in:
>> https://searchfox.org/mozilla-central/source/media/libstagefright/binding/include/mp4parse.h
>>
>> Is this the best current practice that I should follow with encoding_rs?
>>
>> See also:
>> https://users.rust-lang.org/t/how-to-retrieve-h-files-from-dependencies-into-top-level-crates-target/9488
>> (unanswered at the moment)
>
> I don't think we have a best practice for this currently. We hit the
> opposite issue with bindgen, and I've been informed that we need to run
> bindgen at build time because the bindings are ABI-specific. Given that
> the C API here is under your complete control, it seems like it's
> possible to generate a cross-platform header that doesn't have those
> issues, so you could certainly check it in. The only question there is
> how much hassle it will be for you to maintain a checked-in copy.
>
> Alternately you could just generate it at build time, and we could pass
> the path to $(DIST)/include in a special environment variable so you
> could put the header in the right place.
>
> -Ted
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: Should cheddar-generated headers be checked in?

2017-02-22 Thread Ted Mielczarek 
On Wed, Feb 22, 2017, at 07:11 AM, Henri Sivonen wrote:
> Looking at mp4parse, the C header is generated:
> https://searchfox.org/mozilla-central/source/media/libstagefright/binding/mp4parse_capi/build.rs
> But also checked in:
> https://searchfox.org/mozilla-central/source/media/libstagefright/binding/include/mp4parse.h
> 
> Is this the best current practice that I should follow with encoding_rs?
> 
> See also:
> https://users.rust-lang.org/t/how-to-retrieve-h-files-from-dependencies-into-top-level-crates-target/9488
> (unanswered at the moment)

I don't think we have a best practice for this currently. We hit the
opposite issue with bindgen, and I've been informed that we need to run
bindgen at build time because the bindings are ABI-specific. Given that
the C API here is under your complete control, it seems like it's
possible to generate a cross-platform header that doesn't have those
issues, so you could certainly check it in. The only question there is
how much hassle it will be for you to maintain a checked-in copy.

Alternately you could just generate it at build time, and we could pass
the path to $(DIST)/include in a special environment variable so you
could put the header in the right place.

-Ted
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Should cheddar-generated headers be checked in?

2017-02-22 Thread Henri Sivonen 
Looking at mp4parse, the C header is generated:
https://searchfox.org/mozilla-central/source/media/libstagefright/binding/mp4parse_capi/build.rs
But also checked in:
https://searchfox.org/mozilla-central/source/media/libstagefright/binding/include/mp4parse.h

Is this the best current practice that I should follow with encoding_rs?

See also:
https://users.rust-lang.org/t/how-to-retrieve-h-files-from-dependencies-into-top-level-crates-target/9488
(unanswered at the moment)

-- 
Henri Sivonen
hsivo...@hsivonen.fi
https://hsivonen.fi/
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform