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

Implementing file globbing in ParseProject alias. #78

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

Conversation

rpriest1260
Copy link

Please review my changes for file globbing in the parse project alias.
These changes are in support of issue: Project Parser does not support Wildcards. #2183.

Explanation of Changes:
The structure of the project parsing code in Cake.Incubator is considerably different from that which is in the current build of Cake. It seems as though it is moving towards handling most internal functionality as extensions. The changes that i made, I made trying to keep this structure in mind.

To construct a Cake.Core.IO.Globber object, you need to pass in an IFileSystem and an ICakeEnvironment interface. With everything now, being broken down into extension methods (vs. having a ProjectParser object), I went looking for the least disruptive way to get a Globber object down to where I needed it -ProjectParserExtensions.GetNetFrameworkMSBuildProjects() . That was to modify ProjectParserExtensions.ParseProjectFile to accept arguments of IFileSystem and ICakeEnvironment at the end of its argument list. In my opinion it would have been more useful to have some class which retains references to these objects and passed that around. But again, I didn't want to be that disruptive with this change. Or at least, maybe it would be more useful to have a class ProjectParseParams that is accepted by these extension methods (and contains 'configuration', 'platform', 'filesystem', etc.) ? Again, I didn't want to disturb the direction of this module too much.

That said, I am open to discussion and suggestions around these changes.

@wwwlicious
Copy link
Contributor

I think it would be better to isolate this from the main project parsing as it introduces a requirement to access the filesystem. It also doesn't account for the default globs for newer project files I don't think.

There are also issues with includes/excludes/conditions and a variety of other edge cases where the project parser really can't reproduce what the msbuild project system does and that this parser was never intended to try.

I personally haven't come across the need to get file listings during the build process when using this as I need it more for checking references, types etc to route to various tooling. Is there a specific use-case that you wanted the projects file for?

@rpriest1260
Copy link
Author

We have a build system, that performs some pre\post - processing of files. We have certain teams that utilize wildcards in their file inclusions, in their projects. When we parse their projects, it would be quite useful if it just gave back the files that that project would have included in it's build steps. That is our immediate need. However, there have been others who have requested such a feature. If you look take a look at the comments in issue 2183, you will see that @kcamp was also looking for such functionality.

@kcamp also expressed similar concerns in issue 1299 :

"Not sure what I might expect with regard to the results. Attempting to glob/resolve all files included in the pattern might be useful, but not throwing an exception would be the minimum viable"

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.

2 participants