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.

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

(GH-1365) Support architecture folder in tools folder #1512

Merged
merged 2 commits into from
Feb 27, 2018

Conversation

gep13
Copy link
Member

@gep13 gep13 commented Feb 26, 2018

This allows a Chocolatey Package structure similar to the following:

image

Which simply contain:

image

On installation, a .ignore file will be created for the alternative architecture (relative to what is being installed):

image

And a shim will be created pointing at the correct architecture:

image

Fixes #1365

@gep13 gep13 requested a review from ferventcoder February 26, 2018 13:43
foreach (var exeFile in exeFiles)
{
_fileSystem.create_file(exeFile + ".ignore");
}
Copy link
Member

Choose a reason for hiding this comment

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

If there are only 32bit files, what is the expectation here? I think it should probably go ahead and use those files.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ferventcoder good catch. I have updated this PR with a slight modification.

Copy link
Member

Choose a reason for hiding this comment

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

Somehow your whitespace changes also snuck in.

Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

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

Small change requested here.


// If 64bit, and there are only 32bit files, we should shim the 32bit versions,
// therefore, don't ignore anything
if (!exeFiles64Bit.Any() && exeFiles32Bit.Any() && is64Bit)
Copy link
Member

@ferventcoder ferventcoder Feb 27, 2018

Choose a reason for hiding this comment

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

Micro-optimization: You should check is64bit first to short-circuit.

Copy link
Member

@ferventcoder ferventcoder Feb 27, 2018

Choose a reason for hiding this comment

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

You only need to check if there are no 64bit files prior to returning when 64bit.

return;
}

foreach (var exeFile in is64Bit ? exeFiles32Bit : exeFiles64Bit)
Copy link
Member

Choose a reason for hiding this comment

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

Add .or_empty_list_if_null() to the end of those lists (and be sure that is what it is called, I'm using pseudocode).

@gep13
Copy link
Member Author

gep13 commented Feb 27, 2018

@ferventcoder back to you again 😄

Verified

This commit was signed with the committer’s verified signature.
gep13 Gary Ewan Park

Verified

This commit was signed with the committer’s verified signature.
gep13 Gary Ewan Park
- This allows the creation of an x86 or x64 folder in tools
- Shims will be created, depending on identified arch
- No need for install/uninstall script
Copy link
Member

@ferventcoder ferventcoder left a comment

Choose a reason for hiding this comment

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

👍

@ferventcoder ferventcoder merged commit cd8ef68 into chocolatey:stable Feb 27, 2018
@gep13 gep13 deleted the issue-1365 branch February 28, 2018 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants