lib/static-delta: document and check parameters format

[mstuehn]

This enhances the logic handling GVariant parameters within ostree_repo_static_delta_generate(). Several of those entries are expected to be zero-terminated values, and this implicit assumption has been observed to be an hidden trap in languages where strings and arrays may not carry a terminator value (e.g. Rust). In order to improve the situation, this makes the documentation more explicit and actively tries to catch invalid input parameters.

Closes: https://github.com/ostreedev/ostree/issues/2728

---

[Original PR text below here]

This commit adds a test for the rust interface. It should test the static-delta generation into a specified file. Unfortunately it does not work because the ostree-lib throws an error:

---- repo::generate_static::should_generate_static_delta_at stdout ----
thread 'repo::generate_static::should_generate_static_delta_at' panicked at
'static delta generate: Error { domain: g-io-error-quark, code: 1, message: "renameat(./tmp.ReHre1, ): No such file or directory" }', 
rust-bindings/tests/repo/generate_static.rs:39:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

It seems, that there is something wrong with the GVariant of filename, because if (!g_variant_lookup (params, "filename", "^&ay", &opt_filename)) returns "true", but opt_filename is empty (see the empty 2nd parameter of renameat() in the error message).

[openshift-ci[bot]]

Hi @mstuehn. Thanks for your PR.

I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[lucab]

Thanks for the report and the reproducer. I think there is a problem with the filename bytestring variant, which is supposed to be nul-terminated. As you directly transform path -> string -> bytes -> variant through Rust types, there is no terminator in the final value.

If you try using a CString instead it should get past the renaming error:

    let path_var = delta_path
        .to_str()
        .map(std::ffi::CString::new)
        .expect("no valid path")
        .unwrap()
        .as_bytes_with_nul()
        .to_variant();

I do agree this is not very ergonomic.

[lucab]

/ok-to-test

[lucab]

Hey @mstuehn, I did push a couple of commits on top of your PR here. There is now some logic to detect cases where the zero-terminator is missing, and the test has been tweaked to show how to approach it in Rust. Let me know your feedback, but I think we can go ahead and merge this. I think we could gain better UX in the future once we update the bindings for https://github.com/ostreedev/ostree/issues/2647.

[mstuehn]

Hi @lucab the patch looks good to me. Thanks for the merge.