-
Notifications
You must be signed in to change notification settings - Fork 305
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
Merge ostree-rs into this repository #2575
Conversation
Would ostree-rs-ext live under the same |
Yeah, we should figure this out now. Hmm...how about (The thing is this topic really opens the question of the C library hard depending on Rust code, and that adds another wrinkle to this) I guess that could just be |
I like that.
Do you mean like the C code would depend on the
|
No strong opinions on where the bindings live. Having it be It has become less clear to me however whether |
OK a few things here. First a rhel8 branch exists that...well, we could rename it
Yeah. I think I'd like to say that we (the upstream/git main) can, and anyone who can't follow can use the rhel8/2022-branch for now. We (upstream) will still maintain the 2022 branch for stability and security fixes. It's not like we're going to go on a wholesale rewrite anytime soon, so even if we make Rust a hard requirement it's very likely that most things will be easily backportable. |
Look, the type is fine. It's only an opaque thing to ensure lifetimes anyway.
I'm trying to debug a problem in ostree-rs-ext, and it's handy to be able to do `dbg!(mtree.copy_files())`.
The underlying `ostree_repo_load_file()` API has the caller pass `NULL` for output arguments it doesn't want. This isn't sanely bindable in Rust - what the generator does is always request all values, but maps them all to `Option<T>`. The main cases are where a user wants either metadata, or both metadata and content. This API gives just metadata; it's a bit more efficient as we don't need to open the file, and doesn't require the caller to `unwrap()`.
This manually adds a missing `ToGlibPtr` import, which seems to be result of some bugs in `gir` code-generation.
Prep for merge into ostree, where we want to run `cargo fmt` checks in CI.
I want to write APIs that *require* the caller to have set up an ostree transaction. It's natural to require passing a guard to do so. But then we want an accessor for the repo.
Fix up the paths for the crates now that the Rust bindings are in `rust/`. We can't today include the test suite because it depends on `ostree-rs-ext` which would make everything circular. (Building that now requires a separate `cd tests/inst && cargo build`)
OK, I've reworked this to be As of now, this does not make ostree hard depend on Rust. New features may start to depend on Rust. |
It's really tempting to remove `make syntax-check`, it has very very rarely found any real problems. But anyways, just exclude all the binding code because it trips up random problems we simply don't care about like mentions of `O_NDELAY` in the `GLib-2.0.gir`.
`tests/inst` became its own workspace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks sane to me overall. I was expecting to see at least some CI added that builds the bindings.
Need this now that it is it's own workspace.
It should replace our stub one.
We're not using Vagrant or Gitlab, and our container flow is different.
Yeah, fair. Done! |
OK, test run had a new clearly unrelated flake: coreos/fedora-coreos-tracker#1192 There's going to be a lot of followup to this, but let's get this ball rolling! |
/override continuous-integration/jenkins/pr-merge |
@cgwalters: Overrode contexts on behalf of cgwalters: continuous-integration/jenkins/pr-merge In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Followup in ostreedev/ostree-rs#55 |
Starting the ball rolling on #2427
Implemented by following https://stackoverflow.com/questions/1425892/how-do-you-merge-two-git-repositories - specifically
git filter-repo --to-subdirectory-filter rust
. So if you have a different opinion on what the subdirectory should be named (rust/bindings
? in prep for merging ostree-rs-ext too?) now's the time to say!I pushed one small commit on top which fixes up the build system, but that's it so far.