Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Build system handles different feature configurations #353

Open
r1viollet opened this issue Mar 12, 2024 · 3 comments
Open

[FEATURE] Build system handles different feature configurations #353

r1viollet opened this issue Mar 12, 2024 · 3 comments

Comments

@r1viollet
Copy link
Contributor

r1viollet commented Mar 12, 2024

Description

When building libraries in libdatadog, we want to ensure that it is easy to add in or remove modules.
Any logics associated to a module should be within the associated folder.

This means that shell scripts with embedded build logics should be removed.

Example of logics to migrate
https://github.com/DataDog/libdatadog/blob/main/build-telemetry-ffi.sh
https://github.com/DataDog/libdatadog/blob/main/build-profiling-ffi.sh

Package config generation:
https://github.com/DataDog/libdatadog/blob/main/profiling-ffi/datadog_profiling.pc.in

Associated FFI examples:
https://github.com/DataDog/libdatadog/tree/main/examples/ffi

Describe the goal of the feature

The goal of the feature is to be able to do:

cargo build --feature crash-tracking,telemetry,profiling
cargo build --feature trace-pipeline,profiling

Which would produce the expected deliverable.
The associated test commands should work.

Describe alternatives you've considered

We can review the RFC.
The aim is to have a single build for Rust features.

Additional context

As we are looking to add tracing features in the build, we want a clean build system.

@r1viollet r1viollet changed the title [FEATURE] Unify build logics [FEATURE] Build system handles any feature configuration Mar 12, 2024
@r1viollet r1viollet changed the title [FEATURE] Build system handles any feature configuration [FEATURE] Build system handles different feature configurations Mar 13, 2024
@brettlangdon
Copy link
Member

The approach I have been taking in Python is build the Rust FFI/wrappers in the dd-trace-py repo. The goal being I want to be able to pull in specific crates only in our own Cargo.toml instead of worrying about downloading binaries/artifacts.

I've seen the benefit a lot as being able to pick and choose only what I need, as well as building the rust extensions on the same systems that we are building the library on this way we don't have to try and duplicate build environments (os/arch/etc) across multiple repos.

I think this approach should work for most languages. Do you think this is a viable path to achieve the same goals you have? Or are we always going to have to build the FFI/binaries for libdatadog anyways?

https://github.com/DataDog/dd-trace-py/tree/f60060dbed3841244abead9008ff04eb804461af/src/core

@r1viollet
Copy link
Contributor Author

I think this approach should work for most languages. Do you think this is a viable path to achieve the same goals you have? Or are we always going to have to build the FFI/binaries for libdatadog anyways?

https://github.com/DataDog/dd-trace-py/tree/f60060dbed3841244abead9008ff04eb804461af/src/core

Rebuilding from source has some real benefits. I know PHP have chosen this path.

If you are OK with having a Rust toolchain and build times are acceptable, I think this is fine.
What we should try to avoid is having too many ways of delivering Rust to the same language. Some of the logics are tricky to confligure toolchains, build FFIs, publish the right headers, hide some of the symbols. publish artifacts.

Ideally we would document the chosen path (I tried having a RFC, looks like it should evolve per language) and ensure we share practices across a given language.

In the end you still need the feature per feature setting, and you still need the libdatadog CI to test your feature configuration (to avoid having broken feature combinations when folks merge within libdatadog). So it feels like the issue is also relevant to Python.

@r1viollet
Copy link
Contributor Author

@hoolioh coming in with the amazing work in #586 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants