diff mbox series

package/dotnet: add host-dotnet-sdk

Message ID CABWPoFG8k9WccFXRKC6ufRLKDHzdwCyeN3Q1PJZzqpg83fwc1w@mail.gmail.com
State Changes Requested
Headers show
Series package/dotnet: add host-dotnet-sdk | expand

Commit Message

Raul Hidalgo Caballero Dec. 12, 2020, 4:26 p.m. UTC
This adds host-dotnet-sdk , for now it only works on x86_64 but it could
also work for armv5 and aarch64 (Will be done in the future).

Notice that to build  dotnet, you need a working dotnet. This is the reason
for host is just downloading and extracting a precompiled one

Comments

Thomas Petazzoni Jan. 4, 2021, 7:58 a.m. UTC | #1
Hello Raul,

On Sat, 12 Dec 2020 17:26:32 +0100
Raul Hidalgo Caballero <deinok@deinok.com> wrote:

> This adds host-dotnet-sdk , for now it only works on x86_64 but it could
> also work for armv5 and aarch64 (Will be done in the future).
> 
> Notice that to build  dotnet, you need a working dotnet. This is the reason
> for host is just downloading and extracting a precompiled one

Thanks for your contribution!

The first comment is that your patch only adds host-dotnet-sdk, and we
separately had a patch in September 2020 from Andrey Nechypurenko
adding the dotnet-runtime. My understanding is that in fact both are
needed to have complete .NET support in Buildroot: host-dotnet-sdk to
get the compiler on the host machine, and dotnet-runtime to have the
runtime on the target. Is this correct ? If that's the case, then it
would be good to have both patches together in the same series, as well
as a simple test case in support/testing/ that shows it working (we can
provide some help and hints on how to achieve that).

Andrey said he would come up within a new iteration, but I don't think
he has posted any new version in the last few months.

Another comment is that when sending a patch, you should use "git
send-email" so that the patch appears inline, and we can review it
properly. See https://git-send-email.io/. I'm going to copy/paste your
patch below to give some comments. Overall it looks very good, so only
a few small comments here and there.

> From 438c148bbd052a72c39a0e8345ed0e98f00b5044 Mon Sep 17 00:00:00 2001
> From: Raul Hidalgo Caballero <deinok@deinok.com>
> Date: Sat, 12 Dec 2020 17:04:36 +0100
> Subject: [PATCH 1/1] package/dotnet: add host-dotnet-sdk

The commit title should be:

	package/dotnet-sdk: new package

> 
> Signed-off-by: Raul Hidalgo Caballero <deinok@deinok.com>
> ---
>  package/Config.in.host             |  1 +
>  package/dotnet-sdk/dotnet-sdk.hash |  3 +++
>  package/dotnet-sdk/dotnet-sdk.mk   | 19 +++++++++++++++++++
>  package/dotnet/Config.in.host      | 15 +++++++++++++++
>  4 files changed, 38 insertions(+)

You should add an entry in the DEVELOPERS file for this package.

> index 0000000000..714b36c518
> --- /dev/null
> +++ b/package/dotnet-sdk/dotnet-sdk.hash
> @@ -0,0 +1,3 @@
> +sha256	cfc21f5e8bd655ae997eec916138b707b1d290b83272c02a95c9f821b8c87310	LICENSE.txt
> +sha256	01564961f8ca9744d0ecc5d3e72d7c1659df95898f3a077fd9140fd4023f3579	ThirdPartyNotices.txt
> +sha256	23df1eca7eb1302dfb10f4edce7edf7150e57698576f61b2dcb777c833cbd80c	dotnet-sdk-5.0.101-linux-x64.tar.gz

Fields in the hash file should be separated with just two spaces.

> +define HOST_DOTNET_SDK_INSTALL_CMDS
> +	(mkdir -p $(HOST_DIR)/usr/share/dotnet/)
> +	(cp -R $(@D)/* $(HOST_DIR)/usr/share/dotnet/)
> +	(ln -s $(HOST_DIR)/usr/share/dotnet/dotnet $(HOST_DIR)/usr/bin/dotnet)
> +endef

You don't need spaces around those commands. Also, instead of cp -R, we
normally use "cp -dpfr". Also $(HOST_DIR)/usr doesn't really exist,
it's a symlink to $(HOST_DIR). And also perhaps make the link a
relative symlink ?

So:

	mkdir -p $(HOST_DIR)/share/dotnet
	cp -dpfr $(@D)/* $(HOST_DIR)/share/dotnet
	(cd $(HOST_DIR)/bin; ln -s ../share/donet/dotnet)

Do you think you could rework your patch, and perhaps adopt the
dotnet-runtime patch in the same series so that we have a complete
solution ?

Best regards,

Thomas
Andrey Nechypurenko Jan. 4, 2021, 8:53 a.m. UTC | #2
Hi Folks,

> > Notice that to build  dotnet, you need a working dotnet. This is the reason
> > for host is just downloading and extracting a precompiled one
>
> Thanks for your contribution!
>
> The first comment is that your patch only adds host-dotnet-sdk, and we
> separately had a patch in September 2020 from Andrey Nechypurenko
> adding the dotnet-runtime. My understanding is that in fact both are
> needed to have complete .NET support in Buildroot: host-dotnet-sdk to
> get the compiler on the host machine, and dotnet-runtime to have the
> runtime on the target. Is this correct ?

Basically yes. It is correct. Strictly speaking, when
(cross)compiling/packaging .net core application, there are two options:

1. Include all runtime parts (mainly bunch of .so files) with application.

2. Assume that the target platform has .net runtime installed and does not
include relevant parts into the application.

I was focusing on the second option since it prevents runtime duplication
for more than one application (it is not a neglectable amount, in particular
for embedded applications).

> Andrey said he would come up within a new iteration, but I don't think
> he has posted any new version in the last few months.

You are right. I've got comments and improvement suggestions but somehow
this activity was preempted by others with higher priority :-)  Will
definitely try to complete it in the near future.

Regards,
Andrey.
Raul Hidalgo Caballero Jan. 7, 2021, 10:52 p.m. UTC | #3
Hi All,

Glad to hear that the general approach of my patch is okay!!

I will work on the changes requested this weekend.
Probably I will need a bit of help regarding "git commits" as I'm not used
to the email system.

Regarding Andrey's patch,
I'm not sure if that is the correct approach, I mean, the idea is okay for
most of the cases,
but what if you install the dotnet-runtime -> v5.0.0 and the package that
you want to install needs v3.1.0 ?

My idea is to make small steps until dotnet is perfectly integrated with
buildroot, while my knowledge of buildroot grows.
The steps are:
* Add support for host-dotnet-sdk only (via direct download)
* Add all supported host archs
* Add generic package for dotnet using only the host-dotnet-sdk
(self-contained) every package includes it's own runtime
* Add support for dotnet-runtime (via direct download) (already done vi
Andrey)
* Add support for dotnet-aspnetcore (via direct download)
* Add support for not self-contained in the generic package
* Start building target packages using source (this will be a bit hard)
* Start building hos packages using source (this will be a pretty hard)

Best Regards,
HIDALGO CABALLERO, Raul

On Mon, Jan 4, 2021 at 9:53 AM Andrey Nechypurenko <andreynech@gmail.com>
wrote:

> Hi Folks,
>
> > > Notice that to build  dotnet, you need a working dotnet. This is the
> reason
> > > for host is just downloading and extracting a precompiled one
> >
> > Thanks for your contribution!
> >
> > The first comment is that your patch only adds host-dotnet-sdk, and we
> > separately had a patch in September 2020 from Andrey Nechypurenko
> > adding the dotnet-runtime. My understanding is that in fact both are
> > needed to have complete .NET support in Buildroot: host-dotnet-sdk to
> > get the compiler on the host machine, and dotnet-runtime to have the
> > runtime on the target. Is this correct ?
>
> Basically yes. It is correct. Strictly speaking, when
> (cross)compiling/packaging .net core application, there are two options:
>
> 1. Include all runtime parts (mainly bunch of .so files) with application.
>
> 2. Assume that the target platform has .net runtime installed and does not
> include relevant parts into the application.
>
> I was focusing on the second option since it prevents runtime duplication
> for more than one application (it is not a neglectable amount, in
> particular
> for embedded applications).
>
> > Andrey said he would come up within a new iteration, but I don't think
> > he has posted any new version in the last few months.
>
> You are right. I've got comments and improvement suggestions but somehow
> this activity was preempted by others with higher priority :-)  Will
> definitely try to complete it in the near future.
>
> Regards,
> Andrey.
>
Andrey Nechypurenko Jan. 8, 2021, 12:01 p.m. UTC | #4
Hi,

> Regarding Andrey's patch, I'm not sure if that is the correct approach, I
> mean, the idea is okay for most of the cases, but what if you install the
> dotnet-runtime -> v5.0.0 and the package that you want to install needs
> v3.1.0 ?

AFAIK, it is possible to have multiple runtimes installed. There are many
ways to say which one should be used by a particular application. One of
them, for example, using DOTNET_ENVIRONMENT environment variable (
https://docs.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-5.0
) . Also, AFAIK, if the application is packaged with runtime, it will use
this packaged version even if there is separate runtime installed.

Anyway, I think there is no "right way" [tm]. It depends on the application.
For example, in a well controlled environment such as an embedded system
where only a predefined set of applications is installed, common runtime
would be preferable because it saves a considerable amount of space if
multiple applications need it.

Regards,
Andrey.
Raul Hidalgo Caballero Jan. 11, 2021, 11:50 a.m. UTC | #5
Yes, I'm agree. There is no perfect way, we should found an approach that
is perfectly fitted in all cases.

For now, having support for host-dotnet-sdk is a must in any case



On Fri, Jan 8, 2021 at 1:02 PM Andrey Nechypurenko <andreynech@gmail.com>
wrote:

> Hi,
>
> > Regarding Andrey's patch, I'm not sure if that is the correct approach, I
> > mean, the idea is okay for most of the cases, but what if you install the
> > dotnet-runtime -> v5.0.0 and the package that you want to install needs
> > v3.1.0 ?
>
> AFAIK, it is possible to have multiple runtimes installed. There are many
> ways to say which one should be used by a particular application. One of
> them, for example, using DOTNET_ENVIRONMENT environment variable (
>
> https://docs.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-5.0
> ) . Also, AFAIK, if the application is packaged with runtime, it will use
> this packaged version even if there is separate runtime installed.
>
> Anyway, I think there is no "right way" [tm]. It depends on the
> application.
> For example, in a well controlled environment such as an embedded system
> where only a predefined set of applications is installed, common runtime
> would be preferable because it saves a considerable amount of space if
> multiple applications need it.
>
> Regards,
> Andrey.
>
Raul Hidalgo Caballero Jan. 12, 2021, 9:09 a.m. UTC | #6
Hi Thomas,

Made a patch here: https://github.com/deinok/buildroot/pull/1/files
If it looks good I will send it all joined as a .patch file.

Fixed all the issues except for:
Testing -> Do you have an example for testing a host package?


On Mon, Jan 11, 2021 at 12:50 PM Raul Hidalgo Caballero <deinok@deinok.com>
wrote:

> Yes, I'm agree. There is no perfect way, we should found an approach that
> is perfectly fitted in all cases.
>
> For now, having support for host-dotnet-sdk is a must in any case
>
>
>
> On Fri, Jan 8, 2021 at 1:02 PM Andrey Nechypurenko <andreynech@gmail.com>
> wrote:
>
>> Hi,
>>
>> > Regarding Andrey's patch, I'm not sure if that is the correct approach,
>> I
>> > mean, the idea is okay for most of the cases, but what if you install
>> the
>> > dotnet-runtime -> v5.0.0 and the package that you want to install needs
>> > v3.1.0 ?
>>
>> AFAIK, it is possible to have multiple runtimes installed. There are many
>> ways to say which one should be used by a particular application. One of
>> them, for example, using DOTNET_ENVIRONMENT environment variable (
>>
>> https://docs.microsoft.com/en-us/aspnet/core/fundamentals/environments?view=aspnetcore-5.0
>> ) . Also, AFAIK, if the application is packaged with runtime, it will use
>> this packaged version even if there is separate runtime installed.
>>
>> Anyway, I think there is no "right way" [tm]. It depends on the
>> application.
>> For example, in a well controlled environment such as an embedded system
>> where only a predefined set of applications is installed, common runtime
>> would be preferable because it saves a considerable amount of space if
>> multiple applications need it.
>>
>> Regards,
>> Andrey.
>>
>
Thomas Petazzoni Jan. 12, 2021, 2:21 p.m. UTC | #7
Hello Raul,

On Tue, 12 Jan 2021 10:09:05 +0100
Raul Hidalgo Caballero <deinok@deinok.com> wrote:

> Hi Thomas,
> 
> Made a patch here: https://github.com/deinok/buildroot/pull/1/files
> If it looks good I will send it all joined as a .patch file.

Thanks for the updated patch.

Could you please submit as a patch on the Buildroot mailing list, so
that we can give some review/feedback ?

> Fixed all the issues except for:
> Testing -> Do you have an example for testing a host package?

The test case you have is already good. What would be better is to have
the test case use the dotnet SDK to really build an example program,
and then run it on the target.

Best regards,

Thomas
Raul Hidalgo Caballero Jan. 12, 2021, 3:08 p.m. UTC | #8
Done In a new patch, and ready for acceptance.
That kind of testing will be done when i make the generic-package for dotnet

On Tue, Jan 12, 2021 at 3:21 PM Thomas Petazzoni <
thomas.petazzoni@bootlin.com> wrote:

> Hello Raul,
>
> On Tue, 12 Jan 2021 10:09:05 +0100
> Raul Hidalgo Caballero <deinok@deinok.com> wrote:
>
> > Hi Thomas,
> >
> > Made a patch here: https://github.com/deinok/buildroot/pull/1/files
> > If it looks good I will send it all joined as a .patch file.
>
> Thanks for the updated patch.
>
> Could you please submit as a patch on the Buildroot mailing list, so
> that we can give some review/feedback ?
>
> > Fixed all the issues except for:
> > Testing -> Do you have an example for testing a host package?
>
> The test case you have is already good. What would be better is to have
> the test case use the dotnet SDK to really build an example program,
> and then run it on the target.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
Raul Hidalgo Caballero Jan. 19, 2021, 9:23 a.m. UTC | #9
Hi Thomas,

Patch sended, is there any other change I should make?
Link here:
https://patchwork.ozlabs.org/project/buildroot/list/?series=224068


On Tue, Jan 12, 2021 at 4:08 PM Raul Hidalgo Caballero <deinok@deinok.com>
wrote:

> Done In a new patch, and ready for acceptance.
> That kind of testing will be done when i make the generic-package for
> dotnet
>
> On Tue, Jan 12, 2021 at 3:21 PM Thomas Petazzoni <
> thomas.petazzoni@bootlin.com> wrote:
>
>> Hello Raul,
>>
>> On Tue, 12 Jan 2021 10:09:05 +0100
>> Raul Hidalgo Caballero <deinok@deinok.com> wrote:
>>
>> > Hi Thomas,
>> >
>> > Made a patch here: https://github.com/deinok/buildroot/pull/1/files
>> > If it looks good I will send it all joined as a .patch file.
>>
>> Thanks for the updated patch.
>>
>> Could you please submit as a patch on the Buildroot mailing list, so
>> that we can give some review/feedback ?
>>
>> > Fixed all the issues except for:
>> > Testing -> Do you have an example for testing a host package?
>>
>> The test case you have is already good. What would be better is to have
>> the test case use the dotnet SDK to really build an example program,
>> and then run it on the target.
>>
>> Best regards,
>>
>> Thomas
>> --
>> Thomas Petazzoni, CTO, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
>>
>
diff mbox series

Patch

From 438c148bbd052a72c39a0e8345ed0e98f00b5044 Mon Sep 17 00:00:00 2001
From: Raul Hidalgo Caballero <deinok@deinok.com>
Date: Sat, 12 Dec 2020 17:04:36 +0100
Subject: [PATCH 1/1] package/dotnet: add host-dotnet-sdk

Signed-off-by: Raul Hidalgo Caballero <deinok@deinok.com>
---
 package/Config.in.host             |  1 +
 package/dotnet-sdk/dotnet-sdk.hash |  3 +++
 package/dotnet-sdk/dotnet-sdk.mk   | 19 +++++++++++++++++++
 package/dotnet/Config.in.host      | 15 +++++++++++++++
 4 files changed, 38 insertions(+)
 create mode 100644 package/dotnet-sdk/dotnet-sdk.hash
 create mode 100644 package/dotnet-sdk/dotnet-sdk.mk
 create mode 100644 package/dotnet/Config.in.host

diff --git a/package/Config.in.host b/package/Config.in.host
index 4f69785810..c92af79e7e 100644
--- a/package/Config.in.host
+++ b/package/Config.in.host
@@ -16,6 +16,7 @@  menu "Host utilities"
 	source "package/dfu-util/Config.in.host"
 	source "package/dos2unix/Config.in.host"
 	source "package/dosfstools/Config.in.host"
+	source "package/dotnet/Config.in.host"
 	source "package/doxygen/Config.in.host"
 	source "package/dtc/Config.in.host"
 	source "package/e2fsprogs/Config.in.host"
diff --git a/package/dotnet-sdk/dotnet-sdk.hash b/package/dotnet-sdk/dotnet-sdk.hash
new file mode 100644
index 0000000000..714b36c518
--- /dev/null
+++ b/package/dotnet-sdk/dotnet-sdk.hash
@@ -0,0 +1,3 @@ 
+sha256	cfc21f5e8bd655ae997eec916138b707b1d290b83272c02a95c9f821b8c87310	LICENSE.txt
+sha256	01564961f8ca9744d0ecc5d3e72d7c1659df95898f3a077fd9140fd4023f3579	ThirdPartyNotices.txt
+sha256	23df1eca7eb1302dfb10f4edce7edf7150e57698576f61b2dcb777c833cbd80c	dotnet-sdk-5.0.101-linux-x64.tar.gz
diff --git a/package/dotnet-sdk/dotnet-sdk.mk b/package/dotnet-sdk/dotnet-sdk.mk
new file mode 100644
index 0000000000..a32274ea37
--- /dev/null
+++ b/package/dotnet-sdk/dotnet-sdk.mk
@@ -0,0 +1,19 @@ 
+################################################################################
+#
+# dotnet-sdk
+#
+################################################################################
+
+HOST_DOTNET_SDK_VERSION = 5.0.101
+HOST_DOTNET_SDK_SITE = https://dotnetcli.azureedge.net/dotnet/Sdk/$(HOST_DOTNET_SDK_VERSION)
+HOST_DOTNET_SDK_SOURCE = dotnet-sdk-$(HOST_DOTNET_SDK_VERSION)-$(call qstrip,$(BR2_PACKAGE_HOST_DOTNET_RID)).tar.gz
+HOST_DOTNET_SDK_LICENSE = MIT
+HOST_DOTNET_SDK_LICENSE_FILES = LICENSE.txt ThirdPartyNotices.txt
+
+define HOST_DOTNET_SDK_INSTALL_CMDS
+	(mkdir -p $(HOST_DIR)/usr/share/dotnet/)
+	(cp -R $(@D)/* $(HOST_DIR)/usr/share/dotnet/)
+	(ln -s $(HOST_DIR)/usr/share/dotnet/dotnet $(HOST_DIR)/usr/bin/dotnet)
+endef
+
+$(eval $(host-generic-package))
diff --git a/package/dotnet/Config.in.host b/package/dotnet/Config.in.host
new file mode 100644
index 0000000000..90b477a6a1
--- /dev/null
+++ b/package/dotnet/Config.in.host
@@ -0,0 +1,15 @@ 
+config BR2_PACKAGE_HOST_DOTNET_ARCH_SUPPORTS
+	bool
+	default y if BR2_HOSTARCH = "x86_64"
+
+config BR2_PACKAGE_HOST_DOTNET_RID
+	string
+	default "linux-x64" if BR2_HOSTARCH = "x86_64"
+
+config BR2_PACKAGE_HOST_DOTNET_SDK
+	bool "host dotnet-sdk"
+	depends on BR2_PACKAGE_HOST_DOTNET_ARCH_SUPPORTS
+	help
+		Dotnet SDK
+
+		https://dotnet.microsoft.com/
-- 
2.27.0