Skip to content

Conversation

dinhngtu
Copy link
Member

@dinhngtu dinhngtu commented Sep 18, 2025

Add a rewritten XVA bridge getter/setter script using libarchive bindings.

This script streams tar members directly instead of using temporary files, and thus processes XVAs faster than the existing shell scripts, without needing additional disk space (beyond that of the new XVA).

It also uses Python's DOM library to guarantee parsing of ova.xml.

Tested on libarchive-c==5.3. Due to use of library internals, may not work on other versions of libarchive-c.

Delete the old bash scripts for XVA editing.

@dinhngtu dinhngtu requested a review from gduperrey September 18, 2025 14:39
@gduperrey gduperrey requested a review from stormi September 18, 2025 14:59
@gduperrey gduperrey requested a review from glehmann September 18, 2025 15:00
@dinhngtu dinhngtu force-pushed the dnt/fast-xva-bridge branch 6 times, most recently from 6428f16 to c67e179 Compare September 18, 2025 15:44
import logging
from xml.dom import minidom

import libarchive
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the readme mention this requirement? And/or the whole project's python requirements be updated so that everyone can use the script without having to discover the missing dependency at runtime?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure about updating the repo-level requirements since it's not part of the tests, so I'll defer to your preference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @glehmann or @ydirson for opinion. Context: a script which is not directly used by the tests, but is actually a utility we use to generate test XVAs, has a new dependency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, it should be added to the dev dependencies.
uv add --raw 'libarchive-c==5.3'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated deps

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What to do with the old get_xva_bridge.sh and set_xva_bridge.sh? (and the readme which mentions them)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can update the readme. I think the scripts should stay for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For what reason should they stay? The new script is not a full replacement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason, it's rather just in case someone needs them. I can also delete them, since personally I won't be using the old scripts.

Copy link
Member Author

@dinhngtu dinhngtu Sep 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted. But maybe @gduperrey will have a preference since he's the one using this daily?

@dinhngtu dinhngtu force-pushed the dnt/fast-xva-bridge branch 3 times, most recently from e14f860 to 4292c9b Compare September 19, 2025 09:05
@dinhngtu dinhngtu force-pushed the dnt/fast-xva-bridge branch from 4292c9b to ea7e39a Compare October 7, 2025 10:42
@dinhngtu
Copy link
Member Author

dinhngtu commented Oct 7, 2025

Static binary builder using Nuitka is available here: https://github.com/dinhngtu/xva-bridge-builder

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

Successfully merging this pull request may close these issues.

4 participants