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

Update: Ensure we don't create stub for generated stub executables #1355

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IonMiron
Copy link

@IonMiron IonMiron commented Jul 24, 2018

Issue:
If we have a lot of files that are enumerated through the call to GetAllFilesRecursively we end up in the situation where we also enumerate over the newly created stub executables.
From this point two thing happen:

  • we create stub for stub
  • process fails due to file access errors

Fix:
To ensure this does not happen we force a call to ToList() such that we get the list of executables for which to create the stubs.

src/Squirrel.nuspec Outdated Show resolved Hide resolved
src/Update/Program.cs Outdated Show resolved Hide resolved
return rootPath.GetAllFilesRecursively("*");
}

public static IEnumerable<FileInfo> GetAllFilesRecursively(this DirectoryInfo rootPath, string searchPattern)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a function now? It does the same thing

Copy link
Author

Choose a reason for hiding this comment

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

The idea is to have an overload that takes the search pattern as a parameter (to be used when collectiong exe files for stub creation)

@anaisbetts
Copy link
Contributor

What does this do? Can you fill in the description describing what problem this is trying to solve?

@IonMiron
Copy link
Author

Hey @paulcbetts ,
What do you think about the PR? Can you have another look?

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.

3 participants