Skip to content

Add support for git repository sources for nodes#901

Merged
phil-opp merged 104 commits intomainfrom
git-source
Jun 24, 2025
Merged

Add support for git repository sources for nodes#901
phil-opp merged 104 commits intomainfrom
git-source

Conversation

@phil-opp
Copy link
Copy Markdown
Collaborator

@phil-opp phil-opp commented Mar 20, 2025 •
edited
Loading

Adds support for fetching nodes from git repositories through new git, branch, tag, and rev keys in the dataflow YAML file.

TODO:

  • run build commands in dora run too (for consistency)
  • delay node spawn until all nodes have been built
  • Issue: dora build builds all nodes, independent of their target machine
    • Make dora build behave like a dora start without spawning the process -> done in
      edcb7e3
    • send stderr (+ stdout?) of build commands back to CLI
    • proper cleanup of running_dataflows
  • fetch when reusing

Note: This PR updates the dataflow descriptor format and the format of the messages sent between components. This means that users need to update the dora daemon/dora coordinator, dora cli, and dora node API in sync in order to use it.

@phil-opp phil-opp marked this pull request as ready for review April 2, 2025 14:00
@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Apr 2, 2025

@haixuanTao The pip-release CI job fails on armv7 and x86 with a linker error:

error: unable to find dynamic system library 'atomic' using strategy 'no_fallback'

This seems to be related to rust-cross/cargo-zigbuild#252 , which is used by maturin. According to the issue thread, the problem should be fixed already, so I'm not sure why it's still failing. Do you have any idea?

@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Apr 2, 2025 •
edited
Loading

Looks like maturin is still locked to an older version of mimalloc: https://github.com/PyO3/maturin/blob/1331f2512fcc3ebada7f6b4639b0df8810b01231/Cargo.lock#L1299

Edit: I misunderstood their versioning scheme. Looks like the change was already included in the locked version.

@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Apr 3, 2025

I tried some things to fix the linker errors, but nothing worked. I'm out of ideas for now, so any help would be appreciated!

@phil-opp phil-opp added the help wanted Extra attention is needed label Apr 3, 2025
@haixuanTao
Copy link
Copy Markdown
Collaborator

haixuanTao commented Apr 4, 2025

The underlying issue is that libatomic is not well supported on 32bit platform right?

Like this: rust-lang/cargo#13546 ?

And that we depend on system installation of libatomic to support open-ssl right?

Because git requires ssl to work right ? Because you can only pull git https or ssh both requiring ssl.

In which case it seems to me that if libatomic is not well supported on 32bit platform, we can probably feature flag this feature and only make it available on 64bit machine, how does that sound?

@haixuanTao
Copy link
Copy Markdown
Collaborator

haixuanTao commented Apr 10, 2025

So FYI I was able to link some system depency to zig by copying package in /lib/x86_64-linux-gnu/ to $HOME/.rustup/toolchains/1.84-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib

This should make it possible to link dynamic library with zig and finish compilation.

@haixuanTao
Copy link
Copy Markdown
Collaborator

haixuanTao commented Apr 10, 2025

See: ziglang/zig#16733

@phil-opp phil-opp force-pushed the git-source branch 4 times, most recently from 6458c1a to a7c1f04 Compare April 10, 2025 13:40
@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Apr 10, 2025

So FYI I was able to link some system depency to zig by copying package in /lib/x86_64-linux-gnu/ to $HOME/.rustup/toolchains/1.84-x86_64-unknown-linux-gnu/lib/rustlib/x86_64-unknown-linux-gnu/lib

This should make it possible to link dynamic library with zig and finish compilation.

Good idea! I tried to do this with libatomic.so, but the one from lib/x86_64-linux-gnu fails with libatomic.so is incompatible with elf_i386:

ld.lld: error: /home/runner/.rustup/toolchains/1.86-x86_64-unknown-linux-gnu/lib/rustlib/i686-unknown-linux-gnu/lib/libatomic.so is incompatible with elf_i386

I also tried linking/copying /lib/i386-linux-gnu/libatomic.so.1 instead, but this fails again with unable to find dynamic system library 'atomic' using strategy 'no_fallback'.

I don't really know what to do with this information. The directory is in the search path, otherwise copying the x86_64 libatomic wouldn't result in a different error message, but the i386 libatomic seems to be ignored for some reason....

@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Apr 10, 2025

Ah, trying to copy the file instead of symlinking it gave the answer: The /lib/i386-linux-gnu/libatomic.so.1 doesn't exist. I assumed that ln -s would throw an error in this case, but it will happily create a symlink to a non-existing file. Thus, the symlink shows up in the test ls call I did, but it doesn't resolve to anything, so the linker probably ignores it.

I now pushed a commit to install libatomic1-i386-cross, let's see how it goes.

@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Apr 10, 2025

New error message 🎉

💥 maturin failed
  Caused by: Cannot repair wheel, because required library libatomic.so.1 could not be located.

@phil-opp
Fix: Store build results to report them on subsequent WaitForBuild …
eb50a36 
…messages

Quick failures might occur before the CLI even sent the `WaitForBuild` message
@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Jun 18, 2025

Can we put dora-session.yaml in the out folder?

We can, but there might not be an out folder in all cases. The folder is currently created and written by the daemon. So if the demon is running on a different machine, the out folder will be created there. We should probably change this at some point.

@phil-opp
Copy link
Copy Markdown
Collaborator Author

phil-opp commented Jun 20, 2025

Can we put dora-session.yaml in the out folder?

Done in 482a7ec

phil-opp added 3 commits June 20, 2025 16:50
@phil-opp
Convert log records to tracing events
65ded26 
Ensures that `log::info` etc messages are logged too.
@phil-opp
Tweak heuristic for doing local or distributed builds
4392aff
@phil-opp
Improve log output for dora run
bf54c6d 
Send the log messages back to the CLI instead of logging them through tracing at the `daemon` level.
@phil-opp phil-opp mentioned this pull request Jun 21, 2025
Closed
@phil-opp
Run dora executable in release mode for examples
38ff2c8 
Avoids stack overflows on Windows
@haixuanTao
Copy link
Copy Markdown
Collaborator

haixuanTao commented Jun 23, 2025

FYI, double checked on logging coloring and looks good to me!

@phil-opp phil-opp merged commit 76d382a into main Jun 24, 2025
213 of 420 checks passed
@phil-opp phil-opp deleted the git-source branch June 24, 2025 07:01
@phil-opp phil-opp mentioned this pull request Jul 15, 2025
Merged
phil-opp added a commit that referenced this pull request Jul 22, 2025
@phil-opp
Fix: Create working directory if it doesn't exist (#1066)
2f610f9 
We forgot to create the working directory in a few cases in #901. This
PR fixes this by adding the proper `create_dir_all` calls.

Alternative to #1064
@phil-opp phil-opp mentioned this pull request Aug 19, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haixuanTao haixuanTao haixuanTao approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@phil-opp @haixuanTao