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

Uplift how artifacts are handled. #122

Merged
merged 3 commits into from
Feb 28, 2025
Merged

Conversation

stellaraccident
Copy link
Contributor

@stellaraccident stellaraccident commented Feb 28, 2025

Functional changes:

  • Artifact archives now always write their manifest first (which for a tar file is important) and preserve the artifact directory structure (archives were being flattened as I hadn't fully thought through the case).
  • The artifact-flatten command can now operate on exploded artifact directories or artifact archives.
  • Target neutral artifacts are now named syntactically like their targetful counterparts but with a "_generic" suffix (e.g. "foo_dev_gfx100" vs "bar_dev_generic"). When working on Python packaging, I found myself regretting that these were syntactically different, as it made pattern matching harder.
  • While some of this might be possible with just creative use of tar and friends, in my early experience having tried that, there are all kinds of corners with respect to attribute and symlink preservation when I tried to use other tools. I still think this tool is paying for itself because it is platform independent and doesn't have the preservation issues that were silently infecting some other attempts. It also works with trees of hardlinks, which is pretty important for performance of the build.

Additional cleanups:

  • Removes THEROCK_ARTIFACT_ARCHIVE_TYPES: they are always .tar.xz now. This takes out some sketchy zip file handling too, which wasn't working out (zip is a very bad format for this and the numbers were terrible).
  • Adds a unit test for fileset_tool.py. It doesn't have full coverage of everything yet, but tests the artifact->archive->flatten and round trip paths.
  • Removes the configurable --manifest in favor of always generating in a fixed way.

@stellaraccident stellaraccident changed the title Fix artifact archives Uplift how artifact archives are handled. Feb 28, 2025
@stellaraccident stellaraccident changed the title Uplift how artifact archives are handled. Uplift how artifacts are handled. Feb 28, 2025
@stellaraccident stellaraccident marked this pull request as ready for review February 28, 2025 00:36
Copy link
Contributor

@amd-chrissosa amd-chrissosa left a comment

Choose a reason for hiding this comment

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

Those shorthands with pathlib are really cool!

@stellaraccident stellaraccident merged commit 97ab010 into main Feb 28, 2025
2 checks passed
@stellaraccident stellaraccident deleted the fix_artifact_archives branch February 28, 2025 01:48
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.

2 participants