AspView ConfigurationImportStatementsStep broken for VCompile

[Felix Gartsman]

Hello,
The ConfigurationImportStatementsStep pre-compilation step is broken
for non-web applications (vcompile). It uses ConfigurationManager to
read the required section, but it doesn't read the correct web.config.
And since vcompile runs on arbitrary path, the ConfigurationManager
approach cannot work in current implementation.
There are 2 issues here: The system.web section can be in a file
located anywhere, and since it's external to aspview config you need
to be able locate it. I suggest adding a new section to aspview config
and saving it in compiler options. Second issue is accessing
configuration source/compiler options from pre-compilation steps in a
consistent way. It probably will be a breaking change.
If no objections, I'll provide a patch.

[Gauthier Segay]

Hello Felix, it seems the issue for this one is still open:

http://support.castleproject.org/browse/CONTRIB-112

Like you said, it couldn't be bad to separate the imports
configuration parsing code from the compiler step for encapsulation
sake.

As for searching the section at various places, I don't understand it
because I assume we only need to open the web.config at the webroot
(which vcompile knows of) and take what is defined here.

Did you mean something else?

By the way, thanks for pointing this issue back.

[Felix Gartsman]

Sorry, I missed CONTRIB-112 discussion.
VCompile loads the configuration as xml and forgets from where it was.
HOWEVER, the new ICompilationContext solves this, as the compiler
knows where it works.
I propose this: add pre-processor section to aspview config. Inside it
add imports node like references. Parse it to a new
AspViewPreProcessorOptions which will become part of
AspViewEngineOptions and pass it to PreProcessor. Extend
IPreCompilationStep.Process to receive AspViewPreProcessorOptions as
extra parameter (breaking change).
Any other solutions?

[Chadly]

I don't like adding more stuff to the AspView config. The whole idea
behind having AspView load the namespaces from the asp.net config was
to cut down on the amount of duplication in the configuration. In
order to get intellisense in the views to work, you need to add your
namespaces to the pages/namespaces section. If I understand your
suggestion correctly, you are saying that their should be a new
imports section in the AspView config which repeats this information.
I don't like the idea of having to repeat the same info many times in
the config.

We are already repeating assembly references (once in the compilation/
assemblies section -- again to get intellisense to work -- and once in
aspview/reference section), but that's another discussion...

[Ken Egozi]

loading configuration is at the end of the day xml-parsing task.

So, I would be happy with a patch that takes care of loading references and default namespaces from the surrounding web.config, in a way that it would work for both oflline and online compilation modes.

--
Ken Egozi.
http://www.kenegozi.com/blog
http://www.musicglue.com
http://www.castleproject.org
http://www.mamaherb.com
http://www.gotfriends.co.il

[Felix Gartsman]

I wasn't aware regarding the intellisense issue. If so the the only
solution is to make the pre-processor aware of the siteroot and pass a
Configuration object to pre-compilation steps. Offline can never fully
mimic online as it cannot create a merged configuration, but we can
get close enough.

[Ken Egozi]

I am perfectly happy with considering only the main web-config of the site, and leave edge cases out of the loop. I think that the less needed configuration the better.

there could also be a fallback mechanism, that is first look at <aspview/reference> then go to <compilation/assemblies>.

[Felix Gartsman]

How should we allow the IPreCompilationStep get access to the
configuration? Store configuration in IPreProcessor and pass it to
IPreCompilationStep.Process or pass IPreProcessor itself?

[Ken Egozi]

pass it over as part of AspViewCompilerOptions

[Felix Gartsman]

IPreProcessor doesn't have access to it currently, I'll add it as
parameter to IPreCompilationStep.Process.

> >http://www.kenegozi.com/bloghttp://www.musicglue.comhttp://www.castle...

[Felix Gartsman]

Well, the patch is ready. But since IPreCompilationStep interface
changed, I've to modify all tests, or think of another way. Maybe some
global access to current compilation context?

> > Ken Egozi.http://www.kenegozi.com/bloghttp://www.musicglue.comhttp://www.castle...

[Gauthier Segay]

Felix,

first thanks for your willingness to fix this problem, and sorry for
not coming back sooner.

For the patch, my proposition to pass reference is:

- modify precompilation steps constructor that require reference to
AspViewCompilerOptions
- modify DefaultPreCompilationStepsProvider constructor to pass the
AspViewCompilerOptions
- modify static Resolve class PreCompilationStepsProvider property to
a method that take AspViewCompilerOptions as parameter and pass it to
Activator.CreateInstance method

this way, the interface doesn't change, and only precompilation steps
that require something from AspViewCompilerOptions need to be changed,
this seems leaner to me, what do you think?

[Felix Gartsman]

Your idea is much better, so I implemented it in
http://support.castleproject.org/browse/MR-447
Unfortunately, we now get non-deterministic imports from external
configuration, so tests fail when unexpected namespace is imported.
Also fixes VCompile with savefiles=true looking for files in the wrong
place, and re-allows assembly references in internal references node
for backward compatibility.