ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [ant] basil commented on pull request #151: findResources(String, boolean) can unnecessarily search the parent
Date Sun, 25 Jul 2021 18:12:00 GMT

basil commented on pull request #151:
URL: https://github.com/apache/ant/pull/151#issuecomment-886238534


   To summarize, the root cause of this problem is the `|| parent != getParent()` logic and
this associated comment:
   
   > Note: could cause overlaps in case ClassLoader.this.parent has matches and parentHasBeenSearched
is true.
   
   The essence of this PR is just removing that code and comment. Renaming `parentHasBeenSearched`
to `skipParent` is just to improve readability.
   
   @jaikiran seems a bit nervous about this, so let me write up a proof of why this PR is
safe. I dug up the original reason why the offending code was written. In [Ant Bug 30161](https://bz.apache.org/bugzilla/show_bug.cgi?id=30161),
@jglick wrote the following:
   
   > Note that there is a problem with the patch (which is I believe unavoidable unless
you set `CL.parent` correctly, or override `CL.getResources` in JDK 1.5 where it seems to
be nonfinal): if `CL.parent` (whatever that is) does have any matches for the resource name,
and `ACL.parent` has matches for it that are in fact derived ultimately from `CL.parent`,
`ACL.getResources` will produce duplicates. I put in a quick hack to avoid this in case `CL.parent`
== `ACL.parent`. […]
   
   This comment was written in 2004. The problem is that the "quick hack" doesn't work for
Jenkins, which necessitated [Kohsuke's change](https://github.com/jenkinsci/jenkins/commit/9a2882dd70)
ten years later in 2014. Now in 2021, Ant has clearly decided _not_ to set `ClassLoader.parent`,
going the opposite way from Kohsuke (see [the latest comment in Ant Bug 35436](https://bz.apache.org/bugzilla/show_bug.cgi?id=35436)),
but Ant has decided to override `ClassLoader.getResources` in commit 17527b6. The end result
is the same: the "quick hack" is no longer needed.
   
   There's nothing more permanent than a temporary solution.
   
   I have convinced myself that this change is not only safe but desirable both logically
and through empirical testing with Jenkins; feel free to ask me for clarification if you have
any more questions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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


Mime
View raw message