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

d3dmetal: New Port #23886

Merged
merged 1 commit into from
May 13, 2024
Merged

d3dmetal: New Port #23886

merged 1 commit into from
May 13, 2024

Conversation

Gcenx
Copy link
Contributor

@Gcenx Gcenx commented May 10, 2024

Description

A Port for installing Apples D3DMetal, this will be used by game-porting-toolkit & also wine-crossover
The license for this is a little weird so not entirely sure on the best way to handle it. I've set the license to restrictive but that might now be exactly accurate still accord to Nat Brown we can redistribute D3DMetal we just can't use it in commercial wine wrapped game ports.

This port only supports x86_64 on Apple Silicon, the try section is something to ensure it's only installed onto an Apple Silicon system running macOS Sonoma and later systems, if theres a better way to handle that I'm open to ideas.

@kencu & @ryandesign

Type(s)
Tested on

macOS 14.5 23F5074a x86_64
Xcode 15.3 15E204a

Verification
  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@Gcenx Gcenx force-pushed the d3dmetal branch 3 times, most recently from ef73ea8 to f5525f5 Compare May 10, 2024 15:58
@kencu
Copy link
Contributor

kencu commented May 11, 2024

background please.

This is a wine dep? That only runs as Intel on an arm64 mac? And that has to be manually downloaded, and has an uncertain license? And why do we need the game porting toolkit?

Was kinda hoping for a nice, smooth start to this process, but if it truly has to be like this, please spoon feed us what is going on.

@Gcenx
Copy link
Contributor Author

Gcenx commented May 11, 2024

D3DMetal is Apples Direct3D to Metal translation layer that’s used with https://github.com/apple/homebrew-apple/blob/main/Formula/game-porting-toolkit.rb and also used in CrossOver-23.5.0 & CrossOver-24.x (any my Winehq packages can also use it)

This only works on Apple Silicon systems but is only x86_64 you can find pdf versions of the documentation here https://github.com/Gcenx/WineskinServer/tree/master/D3DMetal

This gets used as a replacement for WineD3D and also has builtin functionality for patching out some incompatible functions in CEF when running under wine under Rosetta2.

@Gcenx
Copy link
Contributor Author

Gcenx commented May 11, 2024

And that has to be manually downloaded

It should be possible for macports to host the download on its mirrors.

has an uncertain license?

Apple used a weird license, it can be redistributed but can’t be used in commercial wine wrapped games.

And why do we need the game porting toolkit?

Some developers might want to use game-porting-toolkit in macports instead of brew, I already have a Portfile ready for it.

Etherway well also need this for wine-crossover and possible also for the wine ports.

@Gcenx Gcenx force-pushed the d3dmetal branch 2 times, most recently from a322dc2 to cfc5fa0 Compare May 11, 2024 21:00
@Gcenx Gcenx force-pushed the d3dmetal branch 2 times, most recently from 4601a22 to cffd67f Compare May 11, 2024 21:55
@kencu kencu merged commit cf41ed0 into macports:master May 13, 2024
2 of 8 checks passed
@kencu
Copy link
Contributor

kencu commented May 13, 2024

OK. Let's get this started then, and see where it leads.

@Gcenx
Copy link
Contributor Author

Gcenx commented May 13, 2024

OK. Let's get this started then, and see where it leads.

I guess it’s up to @ryandesign now it add or not to add the needed file to the macOS Sonoma Apple silicon buildbot.

Just need #23887 & #23888 to get merged/packaged then I’ll submit game-porting-toolkit

@Gcenx Gcenx deleted the d3dmetal branch May 13, 2024 05:37
@ryandesign
Copy link
Contributor

I guess it’s up to @ryandesign now it add or not to add the needed file to the macOS Sonoma Apple silicon buildbot.

I don't know anything about this software and don't need to be Cc'd on wine-related things anymore; I've handed them off to you.

Comment on lines +69 to +77
platform darwin i386 {
try {
set is_rosetta2 [exec sysctl -in sysctl.proc_translated]
if { ${is_rosetta2} != 1 } {
ui_error "${name} requires an Apple Silicon mac"
return -code error "unsupported platform"
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You CANNOT error out like this unless in a phase. By erroring out outside of a phase, you're causing unrelated pull requests to fail with the error:

Creating port index in /Users/runner/work/macports-ports/macports-ports/ports
Error: d3dmetal requires an Apple Silicon mac
Failed to parse file devel/d3dmetal/Portfile: unsupported platform

However, if this port requires arm64, then all you need to write in the port is supported_archs arm64. That's it. MacPorts handles the rest.

However, I see you've written supported_archs x86_64; I don't understand that. I don't need to understand it; it's your port and I've absolved myself of the wine-related ports. I just need other unrelated PRs not to fail because of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This port can’t be arm64 as I’d already explained, it’s x86_64 but only supports Apple Silicon.

Then how can this be handled exactly as this can’t only work on Apple Silicon systems and is only x86_64.

Copy link
Contributor

Choose a reason for hiding this comment

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

We've never encountered such a situation before so I have no idea how to handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think I’ve found a solution, I’ll just set it to arm64 when it’s not rosetta2.

It’s strange but it’s not uncommon anymore, there some x86_64 macOS games now that only support Apple Silicon 🤦‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

All I can say is to repeat that you cannot return -code error unless you are in a phase without causing problems. A fix would be to enclose the whole bit in a pre-fetch block for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that, as merged, the port did not build on GitHub Actions, nor will it build on Buildbot, because:

Excluding d3dmetal because it does not support the arm64 arch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that, as merged, the port did not build on GitHub Actions, nor will it build on Buildbot, because:

Excluding d3dmetal because it does not support the arm64 arch

Are depends_run required for building a package with the buildbots?

As this dep would only be needed at install/run time not during the actual build.

Copy link
Contributor

Choose a reason for hiding this comment

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

depends_run dependencies are required to install a port (on any computer, whether under user control or automation).

supported_archs x86_64
description Direct3D to Metal translation layer
long_description {*}${description}
master_sites https://download.developer.apple.com/Developer_Tools/Game_Porting_Toolkit_${version}/Game_Porting_Toolkit_${version}.dmg
Copy link
Contributor

Choose a reason for hiding this comment

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

master_sites is for the URL of the directory that the distfile is in. It does not include the distfile name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This distfile can’t really be fetched directly it’s used in the pre-fetch stage.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I said is still accurate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I’ll deal with that in the next PR to hopefully resolve this then.

@Gcenx
Copy link
Contributor Author

Gcenx commented May 13, 2024

I don't know anything about this software and don't need to be Cc'd on wine-related things anymore; I've handed them off to you.

The only reasons I’d tagged you was to ensure there were no issues with how this Portfile worked.

The other reason was to possibly add the fist file to the macOS Sonoma Apple Silicon buildbot. However I’m not sure if depends_run is actually required when building a package as it’ll be a runtime dep for at least one Portfile.

@Gcenx
Copy link
Contributor Author

Gcenx commented May 13, 2024

The major issues should be resolved with https://github.com/macports/macports-ports/actions/runs/9059082485/job/24885937502?pr=23935

I don’t current have access to any computer so I’m unable to squash commits.

@ryandesign
Copy link
Contributor

It should be possible for macports to host the download on its mirrors.

it can be redistributed

I am not convinced that the license gives us that right.

Also, do I understand correctly that the port installs selected files from the Game Porting Toolkit? I don't think the license allows that either: "The Apple Software is provided as part of a bundle and its components may not be separated from the Apple Software for distribution."

@Gcenx
Copy link
Contributor Author

Gcenx commented May 13, 2024

The port will install the files inside the redist directory, those can be redistributed but can’t be used in commercial wine wrappers ports.

These files are also included in CrossOver for Mac but CodeWeavers can’t use it in there commercial wine wrappers Ports.

If your still hesitant the end-user can handle this themselves, I’ll slightly alter game-porting-toolkit with a note to have them manually install d3dmetal as that is just the wine portion so still falls under the usual LGPL-2.1+ license.

@Gcenx
Copy link
Contributor Author

Gcenx commented May 13, 2024

The major issues should be resolved with https://github.com/macports/macports-ports/actions/runs/9059082485/job/24885937502?pr=23935

I don’t current have access to any computer so I’m unable to squash commits.

#23935

@neverpanic neverpanic mentioned this pull request May 13, 2024
12 tasks
@ReenigneArcher
Copy link

Thanks for fixing this to work on other runners besides macos-14. One question though, how did this even get merged when the CI failed? https://github.com/macports/macports-ports/pull/23886/checks

@Gcenx
Copy link
Contributor Author

Gcenx commented May 13, 2024

Thanks for fixing this to work on other runners besides macos-14. One question though, how did this even get merged when the CI failed? https://github.com/macports/macports-ports/pull/23886/checks

My assumption is Ken assumed the failure was due to it needing to manually download the distfile. I’ve never needed to do any really weird things in any PR before.

This one’s a weird special case where it’s x86_64 but only works on Apple Silicon systems and the only reference I could find for handing something similar was the try block (tweaked to test for Rosetta2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants