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.

Sign up for GitHub

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

Adding x64 emulation support for pkg installers #7893

Merged
merged 3 commits into from
Sep 16, 2021
Merged

Adding x64 emulation support for pkg installers #7893

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77696e6
Adding x64 emulation support for pkg installers
sfoslund Sep 13, 2021
48569a5
PR feedback
sfoslund Sep 16, 2021
762a0c0
PR feedback
sfoslund Sep 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion src/Microsoft.DotNet.Build.Tasks.Installers/src/GenerateMacOSDistributionFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ public class GenerateMacOSDistributionFile : BuildTask
[Required]
public string DestinationFile { get; set; }

public string Alternativex64InstallPath { get; set; }
Copy link
Member

@ericstj ericstj Sep 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What's the use case for this parameter?
Maybe we should expose it in the targets

arcade/src/Microsoft.DotNet.Build.Tasks.Installers/build/bundle.targets

Lines 140 to 145 in e7ede87

<GenerateMacOSDistributionFile
TemplatePath="$(MacOSBundleTemplate)"
ProductBrandName="$(ProductBrandName)"
TargetArchitecture="$(TargetArchitecture)"
BundledPackages="@(_MacOSPackagesToBundle)"
DestinationFile="$(_MacOSDistributionFile)" />

And default it in targets instead of code?

arcade/src/Microsoft.DotNet.Build.Tasks.Installers/build/installer.singlerid.targets

Line 403 in 06b4a81

<_MacOSSharedInstallDir>/usr/local/share/dotnet</_MacOSSharedInstallDir>

That could allow for overriding, that said the current path is a private property set unconditionally, so it's not exactly extensible. Also I don't have a scenario today for any product consuming this SDK that might want to opt out of redirection or set its own path, so maybe we don't even need the parameter.

@jkoritzinsky @NikolaMilosavljevic - can you please make a call on that?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an optional parameter for task, so it can be specified or not specified in calls to this task in a target. AFAIK, defaulting in the targets instead of in the task as it is now would be functionally the same.

Copy link
Member

Choose a reason for hiding this comment

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

Right, but the targets don't expose it, so we could just delete it since folks don't have a way of specifying it today. IMHO we should make both of these paths behave a similar way and be co-located since they address the same concern, but I don't own this codebase so I'd like to hear from the owners. We could always clean this up later since it doesn't impact actual functionality.


public override bool Execute()
{
try
Expand All @@ -38,7 +40,16 @@ public override bool Execute()

var titleElement = new XElement("title", $"{ProductBrandName} ({TargetArchitecture})");

var choiceLineElements = BundledPackages.Select(component => new XElement("line", new XAttribute("choice", component.GetMetadata("FileNameWithExtension"))));
var archScriptContent = @"<![CDATA[
function IsX64Machine() {
var machine = system.sysctl(""hw.machine"");
system.log(""Machine type: "" + machine);
var result = machine == ""x64"" || machine.endsWith(""_x64"");
system.log(""IsX64Machine: "" + result);
return result;
}
]]>";
var scriptElement = new XElement("script", new XText(archScriptContent));

var choiceElements = BundledPackages
.Select(component => new XElement("choice",
Expand All @@ -48,6 +59,23 @@ public override bool Execute()
new XAttribute("description", component.GetMetadata("Description")),
new XElement("pkg-ref", new XAttribute("id", component.GetMetadata("FileNameWithExtension")))));

if (TargetArchitecture == "x64")
{
Alternativex64InstallPath ??= "/usr/local/share/dotnet/x64";

choiceElements =
choiceElements.Select(c => new XElement(c)
.WithAttribute("selected", "IsX64Machine()"))
.Concat(
choiceElements.Select(c => new XElement(c)
.WithAttribute("id", c.Attribute("id").Value + ".alternate")
.WithAttribute("selected", "!IsX64Machine()")
.WithAttribute("customLocation", Alternativex64InstallPath)));
}

var choiceLineElements = choiceElements
.Select(c => new XElement("line", new XAttribute("choice", c.Attribute("id").Value)));

var pkgRefElements = BundledPackages
.Select(component => new XElement("pkg-ref",
new XAttribute("id", component.GetMetadata("FileNameWithExtension")),
Expand Down Expand Up @@ -78,6 +106,7 @@ public override bool Execute()
document.Root.Add(new XElement("choices-outline", choiceLineElements));
document.Root.Add(choiceElements);
document.Root.Add(pkgRefElements);
document.Root.Add(scriptElement);
using XmlWriter writer = XmlWriter.Create(File.OpenWrite(DestinationFile));
document.WriteTo(writer);
}
Expand All @@ -89,4 +118,13 @@ public override bool Execute()
return true;
}
}

static class XElementExtensions
{
public static XElement WithAttribute(this XElement element, XName attribute, object value)
{
element.SetAttributeValue(attribute, value);
return element;
}
}
}