You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by Benson Margulies <bi...@gmail.com> on 2012/05/26 22:00:02 UTC

Looking for advice from shade-y characters on MSHADE-119

The problem here is that we need to relocate /a/b/c to /q/r/s.
However, since relocations are stated in terms of a.b.c form and not
path form, there's no syntax that maps.

Unless, of course, a pattern of a.b.c -> q.r.s should just work for
getClass().getResource("/a/b/c");

It seems reasonable to me, but perhaps I'm missing something? Any
thoughts from deep-shade thinkers would be helpful here.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: Looking for advice from shade-y characters on MSHADE-119

Posted by Dawid Weiss <da...@gmail.com>.
Fyi. I filed a bug with Velocity as well:
https://issues.apache.org/jira/browse/VELOCITY-827

Dawid

On Sat, May 26, 2012 at 11:10 PM, Benson Margulies
<bi...@gmail.com> wrote:
> Velocity has a nasty reputation for problems on the classpath. I've
> fought with it on CXF.
>
> However, if I'm right, the immediate problem is trivial to fix.
>
> In SimpleRelocator, I added this || clause to 'canRelocatePath'. If no
> one pipes up with a reason why this would cause a disaster, I'm going
> to commit it.
>
>        // Allow for annoying option of an extra / on the front of a
> path. See MSHADE-119; comes from
> getClass().getResource("/a/b/c.properties").
>        return path.startsWith( pathPattern ) || path.startsWith ( "/"
> + pathPattern );
>
>
>
> On Sat, May 26, 2012 at 2:05 PM, Dawid Weiss <da...@gmail.com> wrote:
>>> That log message, however, is nuts. Searching in classpath and then
>>> logging that file path? I'd file that as a bug in Velocity.
>>
>> It is definitely an bad code pattern -- should be either
>> class-relative resource or searched via
>> classloader, not class + '/' prefix). I see this from time to time
>> though; it'll be hard to rule out in general and will recur from time
>> to time I bet.
>>
>> Dawid
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: Looking for advice from shade-y characters on MSHADE-119

Posted by Benson Margulies <bi...@gmail.com>.
Velocity has a nasty reputation for problems on the classpath. I've
fought with it on CXF.

However, if I'm right, the immediate problem is trivial to fix.

In SimpleRelocator, I added this || clause to 'canRelocatePath'. If no
one pipes up with a reason why this would cause a disaster, I'm going
to commit it.

        // Allow for annoying option of an extra / on the front of a
path. See MSHADE-119; comes from
getClass().getResource("/a/b/c.properties").
        return path.startsWith( pathPattern ) || path.startsWith ( "/"
+ pathPattern );



On Sat, May 26, 2012 at 2:05 PM, Dawid Weiss <da...@gmail.com> wrote:
>> That log message, however, is nuts. Searching in classpath and then
>> logging that file path? I'd file that as a bug in Velocity.
>
> It is definitely an bad code pattern -- should be either
> class-relative resource or searched via
> classloader, not class + '/' prefix). I see this from time to time
> though; it'll be hard to rule out in general and will recur from time
> to time I bet.
>
> Dawid
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: Looking for advice from shade-y characters on MSHADE-119

Posted by Dawid Weiss <da...@gmail.com>.
> That log message, however, is nuts. Searching in classpath and then
> logging that file path? I'd file that as a bug in Velocity.

It is definitely an bad code pattern -- should be either
class-relative resource or searched via
classloader, not class + '/' prefix). I see this from time to time
though; it'll be hard to rule out in general and will recur from time
to time I bet.

Dawid

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: Looking for advice from shade-y characters on MSHADE-119

Posted by Benson Margulies <bi...@gmail.com>.
On Sat, May 26, 2012 at 1:31 PM, Dawid Weiss <da...@gmail.com> wrote:
> Ideally, this should be configurable via plugins because there is
> simply no way to predict what people may come up with?

As far as I can see, a very simple change to shade allows a/b/c to
match /a/b/c, and I have yet to think of a problem with it.

That log message, however, is nuts. Searching in classpath and then
logging that file path? I'd file that as a bug in Velocity.


>
> The relevant fragment from velocity is not even consistent -- look:
>
> inputStream = getClass().getResourceAsStream("/org/apache/velocity/runtime/defaults/velocity.properties");
> configuration.load(inputStream);
> if(log.isDebugEnabled())
> log.debug("Default Properties File: " + (new
> File("org/apache/velocity/runtime/defaults/velocity.properties")).getPath());
>
> So they use the '/' leading form to lookup the resource but they
> report something else (and a file!, not a resource).
>
> Dawid
>
> On Sat, May 26, 2012 at 10:00 PM, Benson Margulies
> <bi...@gmail.com> wrote:
>> The problem here is that we need to relocate /a/b/c to /q/r/s.
>> However, since relocations are stated in terms of a.b.c form and not
>> path form, there's no syntax that maps.
>>
>> Unless, of course, a pattern of a.b.c -> q.r.s should just work for
>> getClass().getResource("/a/b/c");
>>
>> It seems reasonable to me, but perhaps I'm missing something? Any
>> thoughts from deep-shade thinkers would be helpful here.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org


Re: Looking for advice from shade-y characters on MSHADE-119

Posted by Dawid Weiss <da...@gmail.com>.
Ideally, this should be configurable via plugins because there is
simply no way to predict what people may come up with?

The relevant fragment from velocity is not even consistent -- look:

inputStream = getClass().getResourceAsStream("/org/apache/velocity/runtime/defaults/velocity.properties");
configuration.load(inputStream);
if(log.isDebugEnabled())
log.debug("Default Properties File: " + (new
File("org/apache/velocity/runtime/defaults/velocity.properties")).getPath());

So they use the '/' leading form to lookup the resource but they
report something else (and a file!, not a resource).

Dawid

On Sat, May 26, 2012 at 10:00 PM, Benson Margulies
<bi...@gmail.com> wrote:
> The problem here is that we need to relocate /a/b/c to /q/r/s.
> However, since relocations are stated in terms of a.b.c form and not
> path form, there's no syntax that maps.
>
> Unless, of course, a pattern of a.b.c -> q.r.s should just work for
> getClass().getResource("/a/b/c");
>
> It seems reasonable to me, but perhaps I'm missing something? Any
> thoughts from deep-shade thinkers would be helpful here.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org