Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

Add support for file-glob for file-based Import #79

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

garuma
Copy link
Contributor

@garuma garuma commented Jul 11, 2017

This uses NuGet file globbing library so should support the same set of feature as .nuspec parsing.

This should allow the same level of support than NuGet offers for nuspec files and such
@garuma garuma requested a review from mhutch July 11, 2017 21:34
Copy link
Contributor

@Therzok Therzok left a comment

Choose a reason for hiding this comment

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

Mono.Addins nuspec should also state that Microsoft.Extensions.FileSystemGlobbing is a dependency

@@ -49,6 +49,9 @@
<Reference Include="System.Xml" />
<Reference Include="System.Core" />
<PackageReference Include="NuGet.Build.Packaging" Version="0.1.276" />
<PackageReference Include="Microsoft.Extensions.FileSystemGlobbing">
Copy link
Member

Choose a reason for hiding this comment

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

I to keep Mono.Addins.dll standalone, without other NuGet dependencies. Either copy the globbing code here or use a custom one.

@mhutch
Copy link
Member

mhutch commented Jul 12, 2017

Why not just use MonoDevelop.Addins? That way you can use the full richness of MSBuild, and we don't need to add this to Mono.Addins. You can do this today in your addin csproj:

    <AddinFile Import="Foo/*" />

And the files will be copied to the output directory and added to the manifest.

@mhutch
Copy link
Member

mhutch commented Jul 12, 2017

Also, if this were the approach we were taking, IMO the pack phase should update the manifest to use the expanded glob, so that that basic addin integrity checks still work.

@mhutch
Copy link
Member

mhutch commented Jul 12, 2017

@Therzok what nuspec? :)

Copy link
Member

@mhutch mhutch left a comment

Choose a reason for hiding this comment

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

(see comments)

@slluis
Copy link
Member

slluis commented Jul 13, 2017

I'm not opposed to having this feature, in addition to MonoDevelop.Addins. It may be useful for other apps that can't use the extension.

30 new files to just implement a globbing algorithm is overkill. We can just use msbuild-like globbing, which is probably less complete but good enough. We have an implementation here: https://github.com/mono/monodevelop/blob/5893e486d074ecd8f40bcebe65cd6fe112248245/main/src/core/MonoDevelop.Core/MonoDevelop.Projects.MSBuild/DefaultMSBuildEngine.cs#L823.

Also, if this were the approach we were taking, IMO the pack phase should update the manifest to > use the expanded glob, so that that basic addin integrity checks still work.

That would be nice, although it is not a critical issue. The problem is that files may be imported using attributes or in embedded manifests which we can't easily update when packing.

Base automatically changed from master to main March 9, 2021 14:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants