NAnt integration

[Andrey Shchekin]

I have just implemented a (probably buggy) version of NAnt integration support (and integration support in general).

Working syntax sample:

import tasks from NAnt.Core

target default:

   echo(message : "Hello from NAnt!")

This tries to create absolute minimum of what is needed for NAnt task for execute, so it should be pretty lightweight.

The integration API is MEF-based and detects all available implementations of ITaskImportBuilder in the directory -- so if you do not need NAnt, just remove the library.

Breaking change:

I had to remove @ as symbol syntax and reuse it as an escape character for the keywordish task parameters, so some existing stuff that relied on @ being symbol is probably broken.

However, new IConstructionAware interface allows using classes practically as methods with optional parameters, which seems more fun than dictionaries anyway.

Feel free to integrate it if you think it is an interesting idea, but one note -- I have not wrote any unit-tests for this functionality, which is a shame but a fact.

Any notes on how the naming or architecture should be improved are also welcome.

Andrey

[Jeremy Skinner]

Thanks, this looks great. I'll try and get this merged in sometime over the holidays. I'll take a look at what's broken regarding the @ syntax, but I prefer the IConstructionAware interface anyway so I'll try and re-write the existing helper methods to make use of this. I'll also try and get some basic tests in place.

I haven't started looking at the other issues that you mentioned yet - I'm still trying to iron out issues with copying files with FileList which will hopefully be finished today or tomorrow.

Jeremy

2009/12/22 Andrey Shchekin <ash...@gmail.com>

[Andrey Shchekin]

Ok, I want to polish stuff a bit anyway (for example, IConstructionAware name is not perfect since it is a valid decision to 'run' it again). So maybe I'll change it to I(Auto)Startable or something like that.

Also with all these import options I think having 'include' as a separate keyword is an overcomplication, so I'll probably change 'include' to "import all from 'file'" as I originally planned.

I am also pretty sure NAnt integration *will* have rough edges when used for something nontrivial, so I will use in my main build and look into that.

[Jeremy Skinner]

Great - thanks. Let me know when it's cleaned up and I'll merge it in.

[Jeremy Skinner]

A couple of thoughts after having played with IConstructionAware...

I think IExecutable or IRunnable would probably be a better name.

The really nice thing about IConstructionAware is that each helper method in BuiltIns can now become a separate class. Rather than:

msbuild("foo.sln", { @configuration: "release", @foo: "bar" })

...I can now do this:

msbuild("foo.sln", Configuration: "release", Params: { "foo": "bar" })

...where msbuild is a class by itself rather than a globally-scoped static method. As this is now a regular class, you can also use the "with" syntax if you don't want it all on one line:

with MSBuild("foo.sln"):
  .Configuration = "release"
  .Params = { "foo", "bar" }
  .Constructed()

...and this works without any modification. It's probably also possible to modify "with" to implicitly call .Constructed (or whatever we rename it to) at the end of the With block.

The only thing I don't really like is the removal of @ for symbols. Is it possible to use a different character for escaping? Underscore maybe?

Jeremy

2009/12/22 Jeremy Skinner <jer...@jeremyskinner.co.uk>

[Andrey Shchekin]

Yes, I think IRunnable captures the spirit (and Run looks well with 'with').

One addition though, auto-running does not seem to be semantically the same thing as the ability to be run.

So I would do either IAutoRunnable : IRunnable or [AutoRun] attribute (which looks better but easier to forget about).

It would really be much less work if Boo supported named parameters.

On the other hand, class per task seems to be a good practice anyway.

One nice thing also is that you can process results of tasks using simple syntax:

result = download(url : '...') # some imaginary nant task

print result.httpcode

because by default NAnt task wrappers return themselves after they have been run, 

and tasks communicate results through their properties.

[Andrey Shchekin]

On the symbols question -- I wanted to have '@' because it is obvious escaping choice for people coming from C#.

Since build engine does not require people to learn whole Boo syntax in advance, @ allows for rule of least surprise.

_ is ok, but it is kind of surprising after C# where _ are significant, and some people may use _ for actual variables within a build process.

Neither is described in Boo language manual, that is why I see C#-compatible as a preferable one.

On Tue, Dec 22, 2009 at 5:17 PM, Jeremy Skinner <jer...@jeremyskinner.co.uk> wrote:

[Jeremy Skinner]

Hmm, that's a good point. Still, I do like using @ for symbols....it just makes dictionaries seem cleaner. Wondering which feature would be considered more useful.

2009/12/22 Andrey Shchekin <ash...@gmail.com>

[Andrey Shchekin]

I would formulate this question in a different way)

What exact usages of dictionaries are there in Phantom that can not be done it type-safe way with tasks?

Also there might be another solution to dictionaries, such as anonymous types, for example.

It depends on use case a lot.

[Jeremy Skinner]

Good point...the use properties with tasks largely removes the need for lots of dictionaries. Really the only places that need a dictionary are tasks that take an arbitrary collection of key-value pairs (eg passing parameters through to msbuild with /p:Key=Value).

[Andrey Shchekin]

I think this can be done either by anonymous classes or by interpreting @ in the hash-key-scope as a symbol.

I agree that neither looks good, but maybe plain strings are ok for these cases?

[Jeremy Skinner]

I'm happy with plain strings for these.

[Jeremy Skinner]

Are you happy for me to merge this in, or is there anything else you wanted to do first?

[Andrey Shchekin]

In my build I have stumbled into a crash when includes are combined with "import tasks from".

I'll probably fix it either today or tomorrow depending on when I have time, then it should be ok to merge.

[Andrey Shchekin]

Ok, I fixed the issue, and my build actually works with NAnt task (Migrator.NET).

So it should be safe to merge now.

I still left 'include' as 'include' for now, we can change it later anyway.

[Andrey Shchekin]

One thing I understood yesterday that 'with' sample will not work as-is, since 'with MSBuild("foo.sln")' will run Run immediately after constructor in with. There are two solutions -- either leave it as is or add some filtering parameters to AutoRunRunnablesStep that will allow to filter 'with' from auto-run situations.

On Tue, Dec 22, 2009 at 5:17 PM, Jeremy Skinner <jer...@jeremyskinner.co.uk> wrote:

[Jeremy Skinner]

I'll take a look at modifying this to work properly when inside the with macro.

I've merged your changes into my local repo and one thing I have noticed is that compilation times are significantly slower. On my machine, the full test run now takes 11 seconds while it previously it took 3 seconds...any idea what might be causing this?

Jeremy

2009/12/24 Andrey Shchekin <ash...@gmail.com>

[Andrey Shchekin]

I saw something strange happening there when using includes -- it seems DslEngine compiles more files than it should.

But I am not really sure if it is the same problem.

I think the best way is to use something like dotTrace to check what is slow, I'll look at it today if I have time.

[Andrey Shchekin]

I have one idea about that -- I have made BuildRunner's DslFactory non-static since I originally wanted to pass ITaskImportBuilder collection to BuildRunner (and I think it is not a bad idea to make it IoC-based in the future instead of doing the service location itself).

So in the tests, since you create a lot of build runners, the DslFactory gets re-created a lot of times instead of just once (but for actual build, this has no effect).

Could you try to make DslFactory static again and see if it helps?

[Jeremy Skinner]

Yes - that was it. I hadn't noticed that it was no longer static.

Jeremy

2009/12/26 Andrey Shchekin <ash...@gmail.com>

[Jeremy Skinner]

I've merged merged your changes into my repo along with the following changes:

- Auto-Runnables now work correctly inside a with block. This works, but needs some tidying up as I've duplicated some of AutoRunAllRunnablesStep inside WithMacro.

- Added a base ScriptTest class just to simplify the creation of BuildRunner and PhantomOptions inside tests. This also caches the BuildRunner in a static field rather than having BuildRunner cache the DslFactory internally.

- Removed service location from BuildRunner's ctor to make it ioc-friendly. I added a static factory method for creating a BuildRunner with the defaults.

- Moved the Include support out of the Builtins namespace and into Language.

- Converted msbuild/nant into runnables.

- Updated Phantom's own build script to make use of the new syntax.

I haven't yet included the nant integration in the release package until we decide what to do with the licensing.

Jeremy

2009/12/26 Jeremy Skinner <jer...@jeremyskinner.co.uk>

[Andrey Shchekin]

Great, I'll integrate them into my fork.

As for the licensing, it seems that NAnt uses GPL+Classpath exception.

According to this

http://old.nabble.com/LGPL-vs.-GPL-%2B-Classpath-Exception-to11016036.html

the common understanding is that Classpath exception is confusing, but that the general intent of the exception is similar to LGPL.

Actually, for the dynamically linked libraries, they see very little differences between LGPL and GPL-classpath.

Considering that Java uses Classpath exception for JDK and number of commercial Java application, I would say it is pretty safe to agree with them.

In my opinion, the fact that Phantom license is more clear than NAnt one is an important point in favor of Phantom.