Open jacob-carlborg opened 1 year ago
The current behavior is pretty consistent and makes the most sense for the target.
However, having a constraint to support the host sounds like a good idea for those that need it.
I don't think this makes sense with what we have at the moment and would break packages depending on this / calling commands based on the target.
You should instead check for the platform in the command, e.g. in a shell file or when running D files just checking for version (X86_64)
, etc.
So I would close this issue as wontfix. Commands are supposed to be overhauled a bit for being able to run D files more easily, as well as being able to extend the supported types in the future, although the details aren't specified yet.
I guess it depends how you use the commands and how users think of them. I'm going to guess that most users don't cross-compile and therefore don't make the distinction between "running this command for a target platform" and "running this command where the Dub is invoked, the host platform". The commands will always be run on the host platform, therefore it would make most sense that the platform specific commands would match the host platform and not the target platform.
You should instead check for the platform in the command, e.g. in a shell file or when running D files just checking for version (X86_64), etc.
In my opinion, the most straightforward way to use the commands is to use shell commands. But shell commands are different on different platforms, especially between Windows and Posix. For example, I have this in one of my projects
"preGenerateCommands-posix": ["$PACKAGE_DIR/tools/generate_version.sh"],
"preGenerateCommands-windows": ["$PACKAGE_DIR/tools/generate_version.bat"],
Shell scripts (bash) doesn't run on Windows out of the box and .bat
files don't run at all on Posix. So, I can't even check the target platform in my script because the host platform cannot run the script. In this particular case the script is completely independent of the target platform.
would break packages depending on this / calling commands based on the target.
I did mention that if breaking this would not be acceptable a new syntax could be used. Perhaps:
"preGenerateCommands-host-posix"
I do some cross compiling, as well as lots of platform specific commands in general and almost all of it fits well with the target scheme, but I must admit that the Windows and non-Windows worlds are more or less distinct here (x86/arm64 Windows cross compile on Windows, iOS/Android cross compile on macOS/Linux).
Making the "platform"
attribute (SDL) or platform suffix behave differently here would be inconsistent with the rest and I would strongly oppose that. However, nothing really speaks against using an additional attribute, such as "host"
(SDL). The JSON approach should then also be orthogonal, e.g. with the "host" idea, something like "preGenerateCommands-windows-host-linux"
should also work.
our idea on discord with a new commands syntax was something like
"preGenerateCommands": [
{
"d": "path/to/dfile.d"
},
{
"shell": "path/to/script.sh"
},
"old/syntax.sh"
]
and in that we could also add host platforms and maybe put the platform restriction itself per-command.
This was our idea to solve other issues such as that $DC
might not always support -run
as well as just making it easier in general to invoke D files. We could also pass DUB's D generated things that we already have to the d file that is being run.
System information
Bug Description
When cross-compiling, I think the pre and post commands, i.e.
preGenerateCommands
, should use the host platform instead of the target platform.How to reproduce?
Expected Behavior
The output for the Pre-gen Running commands prints
x86_64
. I think it makes more sense to printaarch64
, since the commands will be running on the host machine. One can use the environment variables$DUB_ARCH
and$DUB_PLATFORM
to get the target platform inside the commands.I guess this is a breaking change and I'm sure someone will rely on the current behavior. If that's not acceptable perhaps a new syntax is required for this.