dub icon indicating copy to clipboard operation
dub copied to clipboard

Cross-compiling should use host platform for pre/post commands

Open jacob-carlborg opened this issue 1 year ago • 5 comments

System information

 $ dub --version
DUB version 1.31.1, built on Mar 12 2023
$ ldc2 --version
LDC - the LLVM D compiler (1.32.0):
  based on DMD v2.102.2 and LLVM 15.0.7
  built with LDC - the LLVM D compiler (1.32.0)
  Default target: arm64-apple-darwin22.5.0
  Host CPU: apple-m1
$ uname -a
Darwin  22.5.0 Darwin Kernel Version 22.5.0: Mon Apr 24 20:53:44 PDT 2023; root:xnu-8796.121.2~5/RELEASE_ARM64_T8103 arm64

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?

$ cat main.d 
/+ dub.sdl:
    name "foo"
    preGenerateCommands "echo aarch64" platform="aarch64"
    preGenerateCommands "echo x86_64" platform="x86_64"
+/

import std;

void main()
{
    writeln("foo");
}
$ dub --arch x86_64 --single main.d                                                                                                   09:47:53
     Pre-gen Running commands for foo
x86_64
    Starting Performing "debug" build using /Users/jacobcarlborg/.dvm/compilers/ldc-1.32.0/bin/ldc2 for x86_64.
    Building foo ~master: building configuration [application]
     Linking foo
     Running foo
foo

Expected Behavior

The output for the Pre-gen Running commands prints x86_64. I think it makes more sense to print aarch64, 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.

jacob-carlborg avatar Jun 20 '23 07:06 jacob-carlborg

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.

rikkimax avatar Jun 20 '23 08:06 rikkimax

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.

WebFreak001 avatar Jun 20 '23 10:06 WebFreak001

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"

jacob-carlborg avatar Jun 20 '23 13:06 jacob-carlborg

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.

s-ludwig avatar Jun 20 '23 13:06 s-ludwig

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.

WebFreak001 avatar Jun 20 '23 14:06 WebFreak001