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

milkytracker: update to 1.03.00. #28224

Closed
wants to merge 1 commit into from
Closed

milkytracker: update to 1.03.00. #28224

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 25, 2021

General

Have the results of the proposed changes been tested?

  • I use the packages affected by the proposed changes on a regular basis and confirm this PR works for me
  • I generally don't use the affected packages but briefly tested this PR

Does it build and run successfully?

(Please choose at least one native build and, if supported, at least one cross build. More are better.)

  • I built this PR locally for my native architecture, (ARCH-LIBC)
  • I built this PR locally for these architectures (if supported. mark crossbuilds):
    • aarch64-musl
    • armv7l
    • armv6l-musl

@ericonr
Copy link
Member

ericonr commented Jan 25, 2021

They are looking for SDL2 in the wrong place for cross, it seems :/

Since it used to work, you probably need to make a patch reverting some commit in their build system.

@ericonr
Copy link
Member

ericonr commented Jan 25, 2021

Maybe milkytracker/MilkyTracker@87d0f55 ? But then we need to fix SDL2 / Cmake instead.

@ghost
Copy link
Author

ghost commented Jan 25, 2021

They are looking for SDL2 in the wrong place for cross, it seems :/

Since it used to work, you probably need to make a patch reverting some commit in their build system.

I'll fix it as soon as possible. I simply need to apply the patch exclusively to the cross targets: the question is how do I patch the cross targets?

@ericonr
Copy link
Member

ericonr commented Jan 25, 2021

Nah, it should be ok to apply it for all.

Still, I'd prefer to figure out what CMake is doing wrong.

@ghost
Copy link
Author

ghost commented Jan 25, 2021

Nah, it should be ok to apply it for all.

Still, I'd prefer to figure out what CMake is doing wrong.

I do not understand why this error pops up in the first place: the source tarball is the same for all targets and yet on non-ARM architectures this builds correctly.

@ericonr
Copy link
Member

ericonr commented Jan 25, 2021

How CMake (and other tools) search for dependencies can change (and have flaws) when cross building. That's the main difference. The upstream commit works on native builds, but it broke something for cross.

@Johnnynator
Copy link
Member

Imo just revert the commit. Fixing the SDL2 provided stuff is far more pain.

@ghost
Copy link
Author

ghost commented Jan 27, 2021

Imo just revert the commit. Fixing the SDL2 provided stuff is far more pain.

Maybe I can restrict the number of architectures in the mean time so at least this package can be pushed forward, then when a fix is found (or upstream fixes this problem), I can push a new revision

@ghost
Copy link
Author

ghost commented Jan 27, 2021

I've opened an issue to upstream, maybe they'll look into it and fix it

@ericonr
Copy link
Member

ericonr commented Jan 27, 2021

I'm not sure upstream will want to fix it. The issue is how SDL2's cmake files are created, because they are using the correct way of finding the dependency.

Have you tried adding a patch that reverts the commit I linked?

@ghost
Copy link
Author

ghost commented Feb 23, 2021

Sorry to say this but I am not using Void Linux anymore so I will not pursue further with this PR. I've also closed these PRs in the meantime: #25410 #20739 #21258 #22969

@ghost ghost closed this Feb 23, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants