Skip to content

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Sep 4, 2025

The other platforms do not use that package as we don't support running containers on them. As such only define the code for linux/freebsd and cause compile errors for other platforms if someone tries to import it there.

@Luap99
Copy link
Member Author

Luap99 commented Sep 4, 2025

cc @mtrmac

@github-actions github-actions bot added the common Related to "common" package label Sep 4, 2025
Copy link

Packit jobs failed. @containers/packit-build please check.

The other platforms do not use that package as we don't support running
containers on them. As such only define the code for linux/freebsd and
cause compile errors for other platforms if someone tries to import it
there.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Copy link
Contributor

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Thanks! Confirming that this does not break anything on macOS.


// O_PATH value on freebsd. We must define O_PATH ourselves
// until https://github.com/golang/go/issues/54355 is fixed.
const O_PATH = 0x00400000 //nolint:staticcheck // ST1003: should not use ALL_CAPS
Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely non-blocking, c/common doesn’t have a stable API, so this ultimately makes no difference: Make this package-private o_Path or oPath, so that it is not a part of the public API of this subpackage?

Copy link
Member Author

Choose a reason for hiding this comment

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

actually we have redefined the exact same openDirectory() function in libpod.

So I think maybe it makes sense to put this into a util pkg instead and then import it in podman

Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t feel strongly about this at all, I’d be fine with merging as is.

Or, maybe, just export c/common/temporary_sys.O_PATH? Either way the caller will need to deal with platforms that don’t support the feature.

Copy link
Member Author

Choose a reason for hiding this comment

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

let's merge it then. I think long term it might be nice to abstract some of these safe path access we need into its own package and try to reuse more of such code across our projects.

@mtrmac mtrmac merged commit 17d8725 into containers:main Sep 8, 2025
28 of 35 checks passed
@Luap99 Luap99 deleted the timezone branch September 8, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
common Related to "common" package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants