Add Scriptcs plugins support #229

laedit · 2015-03-29T13:04:14Z

Thanks to @vikingcode and @glennblock this PR allow Pretzel to have plugins as easily as to create a *.csx file in the _plugins folder.

But like @vikingcode has already said for some parts:

the projects are now upgraded to .NET 4.5
the total size of all dlls / exes is now about 13 mo (we can gain some size if we make a release for Windows (Roslyin) and another for Linux (Mono.Cecil) but I don't know if it worth it)
the merging is no longer supported, ILMerge and ILRepack explodes over a StackOverflowException. I haven't tried Fody.Costura but as stated on Pretzel on Mono #215 it doesn't seems to work for Mono, so between that and the total size of files, I think we should just embrace all the little dlls and stop the merging.

@shiftkey, @JakeGinnivan your thoughts?

glennblock · 2015-03-29T16:44:22Z

Why not have it be an opt-in add on package like Pretzel.ScriptCs. Is that
not possible?
On Sun, Mar 29, 2015 at 6:04 AM Jérémie Bertrand [email protected]
wrote:

Fix #217 #217.

Thanks to @vikingcode https://github.com/vikingcode and @glennblock
https://github.com/glennblock this PR allow Pretzel to have plugins as
easily as to create a *.csx file in the _plugins folder.

But like @vikingcode https://github.com/vikingcode has already said for
some parts:

the projects are now upgraded to .NET 4.5

the total size of all dlls / exes is now about 13 mo (we can gain
some size if we make a release for Windows (Roslyin) and another for Linux
(Mono.Cecil) but I don't know if it worth it)

the merging is no longer supported, ILMerge and ILRepack
https://github.com/gluck/il-repack explodes over a
StackOverflowException. I haven't tried Fody.Costura
https://github.com/Fody/Costura but as stated on Pretzel on Mono #215
Pretzel on Mono #215 it doesn't seems to
work for Mono, so between that and the total size of files, I think we
should just embrace all the little dlls and stop the merging.

@shiftkey https://github.com/shiftkey, @JakeGinnivan

https://github.com/JakeGinnivan your thoughts?

You can view, comment on, or merge this pull request online at:

#229
Commit Summary

Update to .NET 4.5

Add ScriptCsCatalog usage

File Changes

M src/Pretzel.Logic/Pretzel.Logic.csproj
https://github.com/Code52/pretzel/pull/229/files#diff-0 (28)

M src/Pretzel.Logic/packages.config
https://github.com/Code52/pretzel/pull/229/files#diff-1 (10)

M src/Pretzel.Tests/Pretzel.Tests.csproj
https://github.com/Code52/pretzel/pull/229/files#diff-2 (12)

M src/Pretzel.Tests/app.config
https://github.com/Code52/pretzel/pull/229/files#diff-3 (8)

M src/Pretzel.Tests/packages.config
https://github.com/Code52/pretzel/pull/229/files#diff-4 (4)

M src/Pretzel/Pretzel.csproj
https://github.com/Code52/pretzel/pull/229/files#diff-5 (88)

M src/Pretzel/Program.cs
https://github.com/Code52/pretzel/pull/229/files#diff-6 (7)

M src/Pretzel/app.config
https://github.com/Code52/pretzel/pull/229/files#diff-7 (4)

M src/Pretzel/packages.config
https://github.com/Code52/pretzel/pull/229/files#diff-8 (20)

Patch Links:

https://github.com/Code52/pretzel/pull/229.patch

https://github.com/Code52/pretzel/pull/229.diff

—
Reply to this email directly or view it on GitHub
#229.

laedit · 2015-03-29T19:36:40Z

Like always you give great advice, I will try that.

glennblock · 2015-03-29T19:45:26Z

@laedit not always, but at least some of the time :-)

It would seem based on what I know about this, that it should be possible to keep the scriptcs dependency out of Pretzel. Maybe allow passing in a catalog, which means just the MEF dependency is sufficient, or have a non-MEF specific plugin interface.

glennblock · 2015-03-29T19:55:27Z

src/Pretzel/Program.cs

@@ -102,6 +103,7 @@ private void LoadPlugins(string[] commandArgs)
                if (Directory.Exists(pluginsPath))
                {
                    catalog.Catalogs.Add(new DirectoryCatalog(pluginsPath));
+                    catalog.Catalogs.Add(new ScriptCsCatalog(pluginsPath, new ScriptCsCatalogOptions { References = new[] { typeof(DotLiquid.Tag), typeof(Pretzel.Logic.Extensibility.ITag) } }));


Instead of hardcoding the dependency on scriptcs, you could find the catalog and dynamically load it. There's a host of diff strategies you could use to do this. One could be checking if the assembly exists, and if it does you dynamically load it and instantiate yourself via reflection. You could move ScriptcsCatalogOptions to a separate Pretzel.ScriptCs.Contracts package so that Pretzel and the Pretzel.ScriptCs package can reference it, this way Pretzel can still construct and pass in the options easily.

glennblock · 2015-03-29T19:56:06Z

I comment in the PR on one way you might handle this.

On Sun, Mar 29, 2015 at 12:36 PM Jérémie Bertrand [email protected]
wrote:

Like always you give great advice, I will try that.

—
Reply to this email directly or view it on GitHub
#229 (comment).

glennblock · 2015-03-29T19:56:16Z

comment[ed]

On Sun, Mar 29, 2015 at 12:56 PM Glenn Block [email protected] wrote:

I comment in the PR on one way you might handle this.

On Sun, Mar 29, 2015 at 12:36 PM Jérémie Bertrand <
[email protected]> wrote:

Like always you give great advice, I will try that.

—
Reply to this email directly or view it on GitHub
#229 (comment).

laedit · 2015-03-29T21:17:34Z

Since we already use MEF both for internals and plugins I was thinking to use it again, like loading the Pretzel.ScriptCs if exists and include it in the main catalog. Don't know if the DirectoryCatalog with a search pattern likePretzel.ScriptCs.dll will do the trick or if it's better to check and load it manually.
The Pretzel.ScriptCs.Contracts is a great idea, I will keep it in mind thanks.

This reverts commit 440a16a.

JakeGinnivan · 2015-03-30T09:43:15Z

That would actually be quite cool, grab the ScriptCs plugin then ScriptCs extensibility just works.

glennblock · 2015-03-30T19:31:19Z

Yep!

On Mon, Mar 30, 2015 at 2:43 AM Jake Ginnivan [email protected]
wrote:

That would actually be quite cool, grab the ScriptCs plugin then ScriptCs
extensibility just works.

—
Reply to this email directly or view it on GitHub
#229 (comment).

laedit · 2015-03-30T20:36:41Z

I made a quick and dirty test, but I face the same problem:

if I merge all dlls including Pretzel.ScriptCs.Contracts there is a cast exception caused by the class used to transmit parameters to the ScriptCsCatalog: the runtime cannot convert the type from the merged Pretzel assembly to the original Pretzel.ScriptCs.Contracts assembly
if I try to not merge Pretzel.ScriptCs.Contracts, ILMerge throw an exception:
Unresolved assembly reference not allowed: Pretzel.ScriptCs.Contracts.. ILRepack goes to a stack overflow...

Did I miss something?
Or we abandon the merging?

glennblock · 2015-03-31T02:08:58Z

I wasn't suggesting Contracts is merged. Pretzel.ScriptCs.Contracts should
be in a package that Pretzel and Pretzel.ScriptCs both reference. I don't
see how that can work with ILMerge.

On Mon, Mar 30, 2015 at 1:36 PM Jérémie Bertrand [email protected]
wrote:

I made a quick and dirty test, but I face the same problem:

if I merge all dlls including Pretzel.ScriptCs.Contracts there is a
cast exception caused by the class used to transmit parameters to the
ScriptCsCatalog: the runtime cannot convert the type from the merged
Pretzel assembly to the original Pretzel.ScriptCs.Contracts assembly

if I try to not merge Pretzel.ScriptCs.Contracts, ILMerge throw an
exception: Unresolved assembly reference not allowed:
Pretzel.ScriptCs.Contracts.. ILRepack goes to a stack overflow...

Did I miss something?
Or we abandon the merging?

—
Reply to this email directly or view it on GitHub
#229 (comment).

laedit · 2015-03-31T04:09:06Z

That's what I was trying to do in my second point: Pretzel.ScriptCs.Contracts isn't part of the Pretzel merge, but int that case ILMerge can't work.
Or at least I haven't figured out how to make it work.

glennblock · 2015-03-31T06:59:37Z

I know very little about ILMerge but can't be much help her.

If an ILMerged assembly must have all dependencies merged, then it sounds
like you won't be able to split out the catalog functionality.

On Mon, Mar 30, 2015 at 9:09 PM Jérémie Bertrand [email protected]
wrote:

That's what I was trying to do in my second point:
Pretzel.ScriptCs.Contracts isn't part of the Pretzel merge, but int that
case ILMerge can't work.
Or at least I haven't figured out how to make it work.

—
Reply to this email directly or view it on GitHub
#229 (comment).

JakeGinnivan · 2015-03-31T07:03:34Z

@SimonCropp @distantcam any ideas?

Sent from my Windows Phone

From: Glenn Blockmailto:[email protected]
Sent: ý31/ý03/ý2015 07:59
To: Code52/pretzelmailto:[email protected]
Cc: Jake Ginnivanmailto:[email protected]
Subject: Re: [pretzel] Add Scriptcs plugins support (#229)

I know very little about ILMerge but can't be much help her.

If an ILMerged assembly must have all dependencies merged, then it sounds
like you won't be able to split out the catalog functionality.

On Mon, Mar 30, 2015 at 9:09 PM Jérémie Bertrand [email protected]
wrote:

That's what I was trying to do in my second point:
Pretzel.ScriptCs.Contracts isn't part of the Pretzel merge, but int that
case ILMerge can't work.
Or at least I haven't figured out how to make it work.

—
Reply to this email directly or view it on GitHub
#229 (comment).

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/229#issuecomment-87968058.

glennblock · 2015-03-31T07:21:41Z

Thinking about this more I have an idea of how you can do this which won't require Pretzel to reference the shared contracts directly. A new assembly (say Pretzel.ScriptCs) will contain a ScriptCsCatalogFactory (the answer to all things!). The factory will manufacture a ScriptCsCatalog and return it casted as a ComposablePartCatalog. If Pretzel needs to pass in types and such it can do it via the factory. The factory only exposes primitive / framework types, so it can easily be invoked via reflection. Pretzel will dynamically look for the factory and load it and then invoke it to get the catalog.

The code will look something like this...

public static class ScriptCsCatalogFactory {
  public static ComposablePartCatalog Create(string[] scriptArgs, Type[] references)
  {
    var options = new ScriptCsCatalogOptions {...};
    return new ScriptCsCatalog(options);
}

distantcam · 2015-03-31T08:02:49Z

ScriptCs had issues whenever I tried folding it into another assembly. There's a few ScriptCs classes that need to be reworked, and then there was a showstopper in Roslyn that meant I couldn't continue. Having said that I was trying this on the version of ScriptCs that used the old version of Roslyn. I haven't tried it since the update.

glennblock · 2015-03-31T08:05:55Z

Scriptcs still relies on the old version of Roslyn on Windows. You can
install the ScriptCs.Rosyln.Engines module to try the newer Roslyn.
On Tue, Mar 31, 2015 at 1:02 AM Cameron MacFarland [email protected]
wrote:

ScriptCs had issues whenever I tried folding it into another assembly.
There's a few ScriptCs classes that need to be reworked, and then there was
a showstopper in Roslyn that meant I couldn't continue. Having said that I
was trying this on the version of ScriptCs that used the old version of
Roslyn. I haven't tried it since the update.

—
Reply to this email directly or view it on GitHub
#229 (comment).

laedit · 2015-03-31T10:52:35Z

Praise to @glennblock and shame on me. Following your comment I was so focused on the idea of using a Pretzel.ScriptCs.Contracts assembly to pass parameters that I haven't realized that it was the problem and wasn't really needed. Sorry for the fuss.

Just for testing purpose I tried to merge the Pretzel.ScriptCs and all his references (ScriptCs + Engine.Roslyn) but ILMerge still goes to a stack overflow.

And clean program parameters

This reverts commit 372f012.

laedit · 2015-04-21T04:56:09Z

Thanks to the recommendations of @ferventcoder, Pretzel will install itself in "C:\tools\Pretzel" (BinRoot or soon ToolsRoot in Chocolatey world) and so does Pretzel.ScriptCs.
@JakeGinnivan, @shiftkey are you ok with that?

JakeGinnivan · 2015-04-21T08:35:07Z

No objections from me

This reverts commit 440a16a.

And clean program parameters

This reverts commit 372f012.

…nto scriptcs-plugin Conflicts: src/Pretzel.Logic/packages.config

Conflicts: src/Pretzel.Logic/packages.config

Add Scriptcs plugins support

Jérémie Bertrand added 2 commits March 28, 2015 17:40

Update to .NET 4.5

a9fe120

Add ScriptCsCatalog usage

440a16a

glennblock reviewed Mar 29, 2015
View reviewed changes

Revert "Add ScriptCsCatalog usage"

5e4e5c0

This reverts commit 440a16a.

Quick'n dirty test of not depending directly of ScriptCs

ca97bc7

Jérémie Bertrand added 5 commits April 1, 2015 20:04

Clean up

6466ef3

Avoid unnecessary recomposition

7b33564

And clean program parameters

Add install and uninstall scripts

f4e96a8

Little cleanup

0105566

Add uninstall scripts in chocolatey packages

1df2500

laedit mentioned this pull request Apr 15, 2015

Tables and Github Flavoured Markdown in general #237

Closed

Jérémie Bertrand added 2 commits April 16, 2015 20:08

Fix bug when pretzel is used from path

e5ae4bd

Remove unnecessary chocolatey uninstall script

372f012

Jérémie Bertrand added 3 commits April 17, 2015 07:20

Revert "Remove unnecessary chocolatey uninstall script"

6c3dd1c

This reverts commit 372f012.

Fix Pretzel installation folder

ef466f2

Fix chocolatey install / uninstall scripts

aa458d4

Jérémie Bertrand added 21 commits April 23, 2015 18:59

Update to .NET 4.5

b7e21f8

Add ScriptCsCatalog usage

1e33a49

Revert "Add ScriptCsCatalog usage"

e7bf753

This reverts commit 440a16a.

Quick'n dirty test of not depending directly of ScriptCs

d46cb3b

Clean up

74c8e0b

Avoid unnecessary recomposition

94bb673

And clean program parameters

Add install and uninstall scripts

dc6d54b

Little cleanup

87b9aec

Add uninstall scripts in chocolatey packages

d9eb846

Fix bug when pretzel is used from path

d9bbd69

Remove unnecessary chocolatey uninstall script

c2c7cc6

Revert "Remove unnecessary chocolatey uninstall script"

fff9c4e

This reverts commit 372f012.

Fix Pretzel installation folder

029e229

Fix chocolatey install / uninstall scripts

4979ef4

Remove unnecessary chocolatey uninstall script

1fb8371

Revert "Remove unnecessary chocolatey uninstall script"

cb7d222

This reverts commit 372f012.

Merge with master

c0f9c6f

Remove MarkdownDeep Nuget reference

65773cb

Fix some tests

8e8d275

Merge branch 'scriptcs-plugin' of https://github.com/Code52/pretzel i…

92dec15

…nto scriptcs-plugin Conflicts: src/Pretzel.Logic/packages.config

Merge remote-tracking branch 'origin/master' into scriptcs-plugin

976c6b1

Conflicts: src/Pretzel.Logic/packages.config

laedit pushed a commit that referenced this pull request May 5, 2015

Merge pull request #229 from Code52/scriptcs-plugin

883c113

Add Scriptcs plugins support

laedit merged commit 883c113 into master May 5, 2015

laedit deleted the scriptcs-plugin branch May 5, 2015 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Scriptcs plugins support #229

Add Scriptcs plugins support #229

laedit commented Mar 29, 2015

glennblock commented Mar 29, 2015

https://github.com/JakeGinnivan your thoughts?

laedit commented Mar 29, 2015

glennblock commented Mar 29, 2015

glennblock Mar 29, 2015

glennblock commented Mar 29, 2015

glennblock commented Mar 29, 2015

laedit commented Mar 29, 2015

JakeGinnivan commented Mar 30, 2015

glennblock commented Mar 30, 2015

laedit commented Mar 30, 2015

glennblock commented Mar 31, 2015

laedit commented Mar 31, 2015

glennblock commented Mar 31, 2015

JakeGinnivan commented Mar 31, 2015

glennblock commented Mar 31, 2015

distantcam commented Mar 31, 2015

glennblock commented Mar 31, 2015

laedit commented Mar 31, 2015

laedit commented Apr 21, 2015

JakeGinnivan commented Apr 21, 2015

Add Scriptcs plugins support #229

Add Scriptcs plugins support #229

Conversation

laedit commented Mar 29, 2015

glennblock commented Mar 29, 2015

https://github.com/JakeGinnivan your thoughts?

laedit commented Mar 29, 2015

glennblock commented Mar 29, 2015

glennblock Mar 29, 2015

Choose a reason for hiding this comment

glennblock commented Mar 29, 2015

glennblock commented Mar 29, 2015

laedit commented Mar 29, 2015

JakeGinnivan commented Mar 30, 2015

glennblock commented Mar 30, 2015

laedit commented Mar 30, 2015

glennblock commented Mar 31, 2015

laedit commented Mar 31, 2015

glennblock commented Mar 31, 2015

JakeGinnivan commented Mar 31, 2015

glennblock commented Mar 31, 2015

distantcam commented Mar 31, 2015

glennblock commented Mar 31, 2015

laedit commented Mar 31, 2015

laedit commented Apr 21, 2015

JakeGinnivan commented Apr 21, 2015