Skip to content
This repository was archived by the owner on Nov 21, 2018. It is now read-only.

Debian installers #154

Merged
merged 1 commit into from
Jun 15, 2017
Merged

Debian installers #154

merged 1 commit into from
Jun 15, 2017

Conversation

JunTaoLuo
Copy link
Contributor

@JunTaoLuo JunTaoLuo commented Jun 13, 2017 •
edited
Loading

Addresses #149

@JunTaoLuo JunTaoLuo force-pushed the johluo/installers branch from 1625819 to 165f4d7 Compare June 13, 2017 19:38
JunTaoLuo
JunTaoLuo commented Jun 13, 2017
View reviewed changes
tools/debian_config.json
@@ -0,0 +1,30 @@
{
"maintainer_name":"Microsoft",
Copy link
Contributor Author

@JunTaoLuo JunTaoLuo Jun 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@Eilon Please review this debian config file. This is mostly copied over from https://github.com/dotnet/cli/blob/3419a87d6fb620a2ea7981cdd6cd43db3b17cb5f/packaging/deb/dotnet-debian_config.json

Nodes especially important are maintainer_*, maintainer_email, install_root, *_description, homepage, copyright and license.

The package_name and package_version are overridden by the values passed explicitly to the dotnet deb tool. The values in release and control seem to be dummy values though I need to verify them.

@JunTaoLuo JunTaoLuo changed the title Debian and NPM installers Debian and RPM installers Jun 14, 2017
@JunTaoLuo JunTaoLuo changed the title Debian and RPM installers Debian installers Jun 14, 2017
@JunTaoLuo
Copy link
Contributor Author

The generation of deb installers is ready. I'll split the rpm generation into a different PR since that uses a different tool and requires some more work.

JunTaoLuo
JunTaoLuo commented Jun 14, 2017
View reviewed changes
build/repo.targets

<Target Name="BuildRuntimeStoreDeb">
<PropertyGroup>
<RuntimeStoreTimestampDebianPackageName>aspnetcore-store-$(TimestampVersion)</RuntimeStoreTimestampDebianPackageName>
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 follows the CLI naming scheme:

Package name: aspnetcore-store-{version}, in the CLI: https://github.com/dotnet/cli/blob/358568b07f16749108dd33e7fea2f2c84ccf4563/build/package/Installer.DEB.targets#L30
Package file name: aspnetcore-store-{version}-{RID}, in the CLI: https://github.com/dotnet/cli/blob/358568b07f16749108dd33e7fea2f2c84ccf4563/build/Branding.props#L31

natemcmaster
natemcmaster reviewed Jun 14, 2017
View reviewed changes
Copy link
Contributor

@natemcmaster natemcmaster left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This makes me question if aspnet/MetaPackages is the right place for this step. At this point, most of the runtime store generation is so dependent on the layout of coherence-signed output, it seems like this should be coherence-final instead.

build/repo.targets

<!-- Build Docker Image -->
<Exec
Command="docker build --build-arg USER_ID=%24(id -u) -t docker-image-$(InstallerPlatform) $(InstallerPlatform)"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to assert that the docker host is using Linux containers, not windows containers.

$ docker version -f "{{ .Server.Os }}"
linux

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assert added.

build/repo.targets
<Target Name="BuildRuntimeStoreDeb">
<PropertyGroup>
<RuntimeStoreTimestampDebianPackageName>aspnetcore-store-$(TimestampVersion)</RuntimeStoreTimestampDebianPackageName>
<RuntimeStoreNoTimestampDebianPackageName>aspnetcore-store-$(NoTimestampVersion)</RuntimeStoreNoTimestampDebianPackageName>
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we verified the naming of runtimes-store installers with the PMs? //cc @richlander, cref https://github.com/dotnet/designs/issues/2

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'm following the convention highlighted by @DamianEdwards in #124 though I'm not sure if this is correct. That's why I called out on it in my previous comment. This needs to be checked by a PM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@glennc, can you signoff on the naming here?

@JunTaoLuo
Copy link
Contributor Author

@natemcmaster This actually requires an additional configuation on TeamCity that runs after coherence-signed on a Linux machine. We don't have a coherence-final configuration so I'm just using MetaPackages.

@natemcmaster
Copy link
Contributor

We can make new configurations if required. It just seems pointless to have this code in aspnet/Metapackages if it depends on the way coherence-signed works as Coherence-signed is private. It would be better to ensure this code works on its own.

Context: there is a "build from source" effort currently underway to make it easier for people to build asp/.net core on their own.

@JunTaoLuo JunTaoLuo mentioned this pull request Jun 14, 2017
Closed
Eilon
Eilon suggested changes Jun 14, 2017
View reviewed changes
tools/docker/debian/Dockerfile Outdated
@@ -0,0 +1,51 @@
#
# Copyright (c) .NET Foundation and contributors. All rights reserved.
# Licensed under the MIT license. See LICENSE file in the project root for full license information.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update these to Apache.

Eilon
Eilon approved these changes Jun 14, 2017
View reviewed changes
Copy link
Contributor

@Eilon Eilon left a comment

Choose a reason for hiding this comment

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

I'm signing off on the license/links in the PR. Don't know the functionality 😄

@JunTaoLuo
Copy link
Contributor Author

Tested installation and uninstallation using the 16.04 .deb installer. When installed via dpkg, the store will be listed as follows (with other dotnet components as reference):
image

@JunTaoLuo JunTaoLuo requested a review from pranavkm June 14, 2017 21:25
natemcmaster
natemcmaster reviewed Jun 15, 2017
View reviewed changes
tools/RestoreRuntimeStore/RestoreRuntimeStore.csproj Outdated
</PropertyGroup>

<ItemGroup>
<DotNetCliToolReference Include="Build.RS" Version="$(AspNetCoreVersion)" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean PackageReference?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy pasta

natemcmaster
natemcmaster approved these changes Jun 15, 2017
View reviewed changes
Copy link
Contributor

@natemcmaster natemcmaster left a comment

Choose a reason for hiding this comment

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

LGTM

@JunTaoLuo
Copy link
Contributor Author

Got an offline :shipit: from @glennc for the installer names. Will merge soon.

@JunTaoLuo JunTaoLuo force-pushed the johluo/installers branch from 8cfd9ef to f8605a4 Compare June 15, 2017 17:26
@JunTaoLuo JunTaoLuo force-pushed the johluo/installers branch from f8605a4 to 07a227f Compare June 15, 2017 17:28
@JunTaoLuo JunTaoLuo merged commit 07a227f into rel/2.0.0-preview2 Jun 15, 2017
@JunTaoLuo JunTaoLuo deleted the johluo/installers branch June 19, 2017 12:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Eilon Eilon Eilon approved these changes

@natemcmaster natemcmaster natemcmaster approved these changes

@muratg muratg Awaiting requested review from muratg

@mikeharder mikeharder Awaiting requested review from mikeharder

@pranavkm pranavkm Awaiting requested review from pranavkm

Assignees
No one assigned
Labels
cla-already-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JunTaoLuo @natemcmaster @Eilon @dnfclas