Skip to content

Conversation

Luap99
Copy link
Member

@Luap99 Luap99 commented Sep 2, 2025

This reverts commit c45b27f.

This commit was just wrong, local-cross depends on this target as it calls a target like "bin/podman.cross.linux.amd64". Without this it is just broken as there is no matching target.

$ make bin/podman.cross.linux.amd64
make: *** No rule to make target 'bin/podman.cross.linux.amd64'. Stop.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 2, 2025
@Luap99
Copy link
Member Author

Luap99 commented Sep 2, 2025

cc @ninja-quokka

@baude
Copy link
Member

baude commented Sep 2, 2025

makes sense, what do we do with local-cross ?

@Luap99
Copy link
Member Author

Luap99 commented Sep 2, 2025

makes sense, what do we do with local-cross ?

CI I wise nothing, but I need that target locally to ensure we can cross compile all supported targets. useful if you mess around with linux/freebsd only files and you need to know they all compile.
Yes CI will catch cross compile issues but they use a different target, as the jobs are split to speed them up and do not use the catch-all local-cross target.

@baude
Copy link
Member

baude commented Sep 2, 2025

LGTM

@ninja-quokka
Copy link
Collaborator

makes sense, what do we do with local-cross ?

CI I wise nothing, but I need that target locally to ensure we can cross compile all supported targets. useful if you mess around with linux/freebsd only files and you need to know they all compile. Yes CI will catch cross compile issues but they use a different target, as the jobs are split to speed them up and do not use the catch-all local-cross target.

Thanks for the clarification @Luap99, can you add another commit to this PR on top of the revert to improve the comment for this target?

This reverts commit c45b27f.

This commit was just wrong, local-cross depends on this target as it
calls a target like "bin/podman.cross.linux.amd64". Without this it is
just broken as there is no matching target.

$ make bin/podman.cross.linux.amd64
make: *** No rule to make target 'bin/podman.cross.linux.amd64'.  Stop.

Signed-off-by: Paul Holzinger <[email protected]>
To avoid any confusion where people think this target is unused.

Signed-off-by: Paul Holzinger <[email protected]>
Without this the corss binaries will never get rebuild until the user
manually deletes them which is not very useful.

Signed-off-by: Paul Holzinger <[email protected]>
@Luap99
Copy link
Member Author

Luap99 commented Sep 3, 2025

Updated the docs and added another commit to fix the missing SOURCES dep which made it not rebuild on changes.

@ninja-quokka
Copy link
Collaborator

Updated the docs and added another commit to fix the missing SOURCES dep which made it not rebuild on changes.

Perfect, LGTM, only in case you need to push again there is a tiny typo in your last commit message:

"Without this the corss cross binaries"

Copy link
Collaborator

@ninja-quokka ninja-quokka left a comment

Choose a reason for hiding this comment

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

/LGTM

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2025
Copy link
Contributor

openshift-ci bot commented Sep 3, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99, ninja-quokka

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 5e5f0a1 into containers:main Sep 3, 2025
85 of 89 checks passed
@Luap99 Luap99 deleted the fix-local-cross branch September 3, 2025 11:17
@Luap99
Copy link
Member Author

Luap99 commented Sep 4, 2025

FYI, I needed this target for testing 2c89069
I have to be able to test stuff locally to not break embargo's.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants