Re: [dev-servo] making features optional
On 21.12.2017 06:20, Fabrice Desre wrote: Hi, I feature-gated the bluetooth support in > https://github.com/fabricedesre/servo/commit/2afbaf365f6379eedf26e36a33e7cb38aba5c00a#diff-8c43e49d8f1af13938ed86125b97244c > > There are more changes that really needed because of rustfmt that ran > when I saved files :( but look for the web-bluetooth feature. Maybe we should run rustfmt once on the whole tree and then put it into pre-commit-hook, to get consistent formatting automatically ? In components/servo/lib.rs, you didn't disable bluetooth_traits::BluetoothRequest. What about the references in the Navigator JSON object ? I'd like to make several features optional, which I'll never need >> and don't want on my systems (eg. bluetooth, webvr, ...). I've already started w/ adding conditional compiles, but that doesn't>> catch everything (and i don't like to patches w/ hard removals): * IDL code generator currently processes all *.webidl files,>> no explicit lists that can be created on build options>> * webidl doesn't have any preprocessor/conditional compile yet> > I don't know how much of that codegen is driven by cargo vs. mach. I > think cargo should let us do that properly. I'm pretty fresh w/ rust+cargo ... can you give me some hint how to pass the chosen features down to the generator python script ? * cargo seems to compile all *.rs files - no explicit lists, that can be made depending on features. using #[cfg(..)] let you do that in general by selecting the modules you need. Does it only compile those modules that are actually imported ? In components/script/dom/mod.rs I see several vr* modules imported, but not actually used. Is that for getting them compiled ? Do they register themselves automatically ? Is that what #[dom_struct] does ? Can we move these imports to a separate module, so we'd need only #[cfg(...)] for that ? * some code pieces (eg. function call parameters) cant be compiled conditionally yet Not sure what you mean here. Can you elaborate? For sure sometimes there will be some refactoring needed (my patch should turn the IpcSender into Option> for instance) but that doesn't seem like a huge issue. Yeah, the new() function takes a reference to BluetoothRequest, therefore this needs to be defined in the first place - which I had cut out. Note, I removed the bluetooth rs files, to make sure they won't be compiled anywhere. Unlike me, you didn't drop the dependencies to 'bluetooth' and 'bluetooth_traits' dependencies - wouldn't that mean they're still compiled ? Or is rustc clever enough to skip them entirely ? BTW: I'd also like to make the script crate smaller, as it doesn't compile on my 32bit environment (takes >4G process size). --mtx ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] making features optional
On 12/21/2017 11:12 AM, google via dev-servo wrote: On 21.12.2017 06:20, Fabrice Desre wrote: Hi, I feature-gated the bluetooth support in > https://github.com/fabricedesre/servo/commit/2afbaf365f6379eedf26e36a33e7cb38aba5c00a#diff-8c43e49d8f1af13938ed86125b97244c > > There are more changes that really needed because of rustfmt that ran > when I saved files :( but look for the web-bluetooth feature. Maybe we should run rustfmt once on the whole tree and then put it into pre-commit-hook, to get consistent formatting automatically ? I'd like that, but I'm quite sure that there are cases where we want hand tuned formatting so doing it properly will be time consuming. In components/servo/lib.rs, you didn't disable bluetooth_traits::BluetoothRequest. What about the references in the Navigator JSON object ? Indeed this is a quick hacked patch to unblock me on something else. Absolutely not ready to be submitted in any way. I'd like to make several features optional, which I'll never need >> and don't want on my systems (eg. bluetooth, webvr, ...). I've already started w/ adding conditional compiles, but that doesn't>> catch everything (and i don't like to patches w/ hard removals): * IDL code generator currently processes all *.webidl files,>> no explicit lists that can be created on build options>> * webidl doesn't have any preprocessor/conditional compile yet> > I don't know how much of that codegen is driven by cargo vs. mach. I > think cargo should let us do that properly. I'm pretty fresh w/ rust+cargo ... can you give me some hint how to pass the chosen features down to the generator python script ? I don't know yet how the binding generator is hooked up in the build system, so I can't answer that. * cargo seems to compile all *.rs files - no explicit lists, that can be made depending on features. using #[cfg(..)] let you do that in general by selecting the modules you need. Does it only compile those modules that are actually imported ? Yes, if you have: #[cfg(feature = "some-api")] mod some_api_impl; then some_api_impl.rs will only be compiled when the feature is set. Same with crates. In components/script/dom/mod.rs I see several vr* modules imported, but not actually used. Is that for getting them compiled ? Do they register themselves automatically ? Is that what #[dom_struct] does ? Can we move these imports to a separate module, so we'd need only #[cfg(...)] for that ? That's possible, I haven't looked into that. Yeah, the new() function takes a reference to BluetoothRequest, therefore this needs to be defined in the first place - which I had cut out. Note, I removed the bluetooth rs files, to make sure they won't be compiled anywhere. Unlike me, you didn't drop the dependencies to 'bluetooth' and 'bluetooth_traits' dependencies - wouldn't that mean they're still compiled ? Or is rustc clever enough to skip them entirely ? bluetooth_traits is still compiled, yes. I think bluetooth is not anymore since my issue was to not bring in the android impl that doesn't work on my android-like-but-not-really platform. Overall, it's absolutely possible to do that cleanly and to compile only what's needed. As I said earlier, that was just not my immediate goal. BTW: I'd also like to make the script crate smaller, as it doesn't compile on my 32bit environment (takes >4G process size). well, good luck with that... compiling servo with less than 16G on a 64bits arch is a challenge. I honestly don't see the point of making compiling on a 32bits host a priority. Fabrice ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
[dev-servo] RFC: (semi-)automatic source formatting ?
On 22.12.2017 05:57, Fabrice Desre wrote: I'd like that, but I'm quite sure that there are cases where we want hand tuned formatting so doing it properly will be time consuming. Perhaps do that in smaller steps, for individual subtrees ? @all: how do you folks think about that ? BTW: which is the actual command you used ? --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287 ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] RFC: (semi-)automatic source formatting ?
On 12/21/2017 09:02 PM, Enrico Weigelt, metux IT consult via dev-servo wrote: On 22.12.2017 05:57, Fabrice Desre wrote: I'd like that, but I'm quite sure that there are cases where we want hand tuned formatting so doing it properly will be time consuming. Perhaps do that in smaller steps, for individual subtrees ? @all: how do you folks think about that ? BTW: which is the actual command you used ? I use Visual Studio Code with the RLS, and it runs rusfmt when saving. So, whatever the default for the RLS are! ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] making features optional
On 22.12.2017 05:57, Fabrice Desre wrote: In components/servo/lib.rs, you didn't disable >> bluetooth_traits::BluetoothRequest. What about the references in the Navigator JSON object ?> > Indeed this is a quick hacked patch to unblock me on something else. > Absolutely not ready to be submitted in any way. Seems it's pretty unfinished yet. Maybe it effectively disables the feature (which a pref also would do), but IMHO most of the code is still in there. But my goals getting rid of everything I don't need/want. One thing that's bothering me is that these things seem to be a bit tricky to decouple due to lack of forward declarations - in C/C++ we could just use forward declarations everywhere we just need a pointer to some struct/class and include the actual definition only where we actually access it - in that case the allocator of some compiled- out class could just be a NULL-returning dummy ... Any idea how to solve this rust ? I don't know yet how the binding generator is hooked up in the build > system, so I can't answer that. @all: anybody else could give us a hint ? Does it only compile those modules that are actually imported ? Yes, if you have: #[cfg(feature = "some-api")] mod some_api_impl; then some_api_impl.rs will only be compiled when the feature is set. Same with crates. Okay. Is there any debug flag for listing which files are actually compiled ? I'd like to verify that ... Do they register themselves automatically ? Is that what #[dom_struct] does ? Can we move these imports to a separate module, so we'd need only #[cfg(...)] for that ? That's possible, I haven't looked into that. @all: maybe anybody else could give us more insight ? bluetooth_traits is still compiled, yes. I think bluetooth is not anymore since my issue was to not bring in the android impl that doesn't work on my android-like-but-not-really platform. Which platform exactly is that ? well, good luck with that... compiling servo with less than 16G on a 64bits arch is a challenge. I honestly don't see the point of making compiling on a 32bits host a priority. I currently only have a 32bit environment, and don't like set up an 64bit container (already a little low of storage space :o) --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering i...@metux.net -- +49-151-27565287 ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
Re: [dev-servo] making features optional
On 12/21/2017 09:24 PM, Enrico Weigelt, metux IT consult via dev-servo wrote: Seems it's pretty unfinished yet. Maybe it effectively disables the feature (which a pref also would do), but IMHO most of the code is still in there. But my goals getting rid of everything I don't need/want. Again, I never claim my patch was complete. It's a decent temporary hack, nothing more. One thing that's bothering me is that these things seem to be a bit tricky to decouple due to lack of forward declarations - in C/C++ we could just use forward declarations everywhere we just need a pointer to some struct/class and include the actual definition only where we actually access it - in that case the allocator of some compiled- out class could just be a NULL-returning dummy ... Any idea how to solve this rust ? That's because you think with a c/c++ mindset. Things are different in Rust, I'm quite sure we can do something nice with traits. bluetooth_traits is still compiled, yes. I think bluetooth is not anymore since my issue was to not bring in the android impl that doesn't work on my android-like-but-not-really platform. Which platform exactly is that ? It's Gonk. Fabrice ___ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
