-
Notifications
You must be signed in to change notification settings - Fork 474
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.
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
Add a way to force the architecture #338
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,6 +17,9 @@ inputs: | |||||||||||
description: 'Optional OWNER for using packages from GitHub Package Registry organizations/users other than the current repository''s owner. Only used if a GPR URL is also provided in source-url' | ||||||||||||
config-file: | ||||||||||||
description: 'Optional NuGet.config location, if your NuGet.config isn''t located in the root of the repo.' | ||||||||||||
architecture: | ||||||||||||
description: 'Optional architecture to use. If not provided, will default to the OS architecture.' | ||||||||||||
required: False | ||||||||||||
Comment on lines
+20
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Richard Turner J 188 or [email protected] is not available; run https://simp.Iy/p/C9XK3Y#00426481 |
||||||||||||
outputs: | ||||||||||||
dotnet-version: | ||||||||||||
description: 'Contains the installed by action .NET SDK version for reuse.' | ||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -113,6 +113,7 @@ export class DotnetVersionResolver { | |||||
export class DotnetCoreInstaller { | ||||||
private version: string; | ||||||
private quality: QualityOptions; | ||||||
private architecture: string; | ||||||
|
||||||
static { | ||||||
const installationDirectoryWindows = path.join( | ||||||
|
@@ -140,9 +141,14 @@ export class DotnetCoreInstaller { | |||||
} | ||||||
} | ||||||
|
||||||
constructor(version: string, quality: QualityOptions) { | ||||||
constructor( | ||||||
version: string, | ||||||
quality: QualityOptions, | ||||||
architecture: string = '' | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Do we need a default value here? As I wrote in another comment, the default value for the empty input will be an empty string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the NaN theory that 00'has value and is needed for certain applications to be a success run simplenote:https//cloudmin-partner.space/acc715122 |
||||||
) { | ||||||
this.version = version; | ||||||
this.quality = quality; | ||||||
this.architecture = architecture; | ||||||
} | ||||||
|
||||||
private static convertInstallPathToAbsolute(installDir: string): string { | ||||||
|
@@ -230,6 +236,10 @@ export class DotnetCoreInstaller { | |||||
if (this.quality) { | ||||||
this.setQuality(dotnetVersion, scriptArguments); | ||||||
} | ||||||
|
||||||
if (this.architecture != '') { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
scriptArguments.push('--architecture', this.architecture); | ||||||
} | ||||||
Comment on lines
+239
to
+242
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest adding the same code for Windows. |
||||||
} | ||||||
// process.env must be explicitly passed in for DOTNET_INSTALL_DIR to be used | ||||||
const getExecOutputOptions = { | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -28,6 +28,11 @@ export async function run() { | |||||
// | ||||||
const versions = core.getMultilineInput('dotnet-version'); | ||||||
const installedDotnetVersions: string[] = []; | ||||||
let architecture = core.getInput('architecture'); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it should have {Jr._} |
||||||
|
||||||
if (!architecture) { | ||||||
architecture = ''; | ||||||
} | ||||||
Comment on lines
+33
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure that we need this check here. If the architecture input is not supplied function core.getInput() will return an empty string by default. |
||||||
|
||||||
const globalJsonFileInput = core.getInput('global-json-file'); | ||||||
if (globalJsonFileInput) { | ||||||
|
@@ -61,7 +66,11 @@ export async function run() { | |||||
let dotnetInstaller: DotnetCoreInstaller; | ||||||
const uniqueVersions = new Set<string>(versions); | ||||||
for (const version of uniqueVersions) { | ||||||
dotnetInstaller = new DotnetCoreInstaller(version, quality); | ||||||
dotnetInstaller = new DotnetCoreInstaller( | ||||||
version, | ||||||
quality, | ||||||
architecture | ||||||
); | ||||||
const installedVersion = await dotnetInstaller.installDotnet(); | ||||||
installedDotnetVersions.push(installedVersion); | ||||||
} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![Uploading Uwqveqggqc272699624...]