Bug 18391 - RFE : ignoreMissingBuildFiles option for subant
Summary: RFE : ignoreMissingBuildFiles option for subant
Status: REOPENED
Alias: None
Product: Ant
Classification: Unclassified
Component: Core tasks (show other bugs)
Version: 1.7.0
Hardware: Other other
: P3 enhancement (vote)
Target Milestone: ---
Assignee: Ant Notifications List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2003-03-26 21:29 UTC by Gus Heck
Modified: 2009-07-30 07:38 UTC (History)
0 users



Attachments
A patch to add ignoreMissingBuildFiles atribute to subant (1.70 KB, patch)
2003-03-26 21:30 UTC, Gus Heck
Details | Diff
Since I had to re-add this for my build to work with 1.7alpha, here is the patch recreated vs the latest CVS (1.60 KB, patch)
2003-09-24 16:21 UTC, Gus Heck
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Gus Heck 2003-03-26 21:29:31 UTC
I was just fooling around with subant, and became really annoyed that it failed
every time it couldn't find a build file, but I definately wanted it to fail if
any of the subbuilds failed. 

I decided it was faster and prevented future maintainence to patch subant than
specifically exclude directories that happened to live at the same level, but
don't yet have build files or don't need them. 

So the request is for the addition of a ignoreMissingBuildFiles atribute for
subant. It should of course default to false to preserve the existing behavior,
and when true cause subant to simply ignore any specified build file that is not
found. (thus ignoring both a missing (default) build.xml, and any explicit build
file that doesn't exist) so here comes the patch...
Comment 1 Gus Heck 2003-03-26 21:30:21 UTC
Created attachment 5523 [details]
A patch to add ignoreMissingBuildFiles atribute to subant
Comment 2 Dominique Devienne 2003-03-26 21:34:39 UTC
Hmmm, I never thought that would be an issue, as I've never setup a buildpath 
that includes sub-projects without a build file, but I guess it could be useful 
to some people (obviously ;-). I would definitely keep the current default 
behavior though, as you indicated you did in your patch. --DD
Comment 3 Steve Loughran 2003-03-26 21:43:58 UTC
I'd have expected the project to take a fileset, and you'd just say
server/*/build.xml. But I get the implication that directories without a
build.xml fail? That's cheesy. 

Given this task is in its infancy, I'm happy with not only the change, but any
underlying behaviour changes we need (i.e. get the defaults right)
Comment 4 Gus Heck 2003-03-26 21:48:01 UTC
Actually I was giving it a DirSet :) and letting it just look for build.xml in
each directory. Seems to work great on my machine with my patch applied.
Comment 5 Dominique Devienne 2003-03-26 21:51:08 UTC
I'm not following Steve... What does cheesy means in this context?

<subant> can take a <fileset> or a <dirset> or whatever else a path can take. 
java.io.Files which are not build files (i.e. !isFile() || isDirectory()) are 
appended the default build file name ('antfile' attribute, same as <ant> BTW). 
Of course, like in most places in Ant, explicitly telling a task to do 
something that missing the essential processing unit, the build file, is an 
error by default in <subant>.

What is wrong with that? I guess I'm not following... --DD
Comment 6 Dominique Devienne 2003-03-26 21:54:25 UTC
Then you could have simply specified a fileset...

Assuming:

<dirset dir="${dir}">
  <include name="${somepattern}"/>
</dirset>

using 

<fileset dir="${dir}">
  <include name="${somepattern}/build.xml"/>
</dirset>

Would only have found the *existing* build files. Not need for a patch in this 
case, and still as order less as before. --DD
Comment 7 Gus Heck 2003-03-26 22:09:01 UTC
I agree with the current default. I can see utility in both directions. In my
case I am willing to commit to being sure that the build file is there, because
the target I am writing is for develop-time compiling of multiple packages that
rely on a library (which I am going to edit and don't want to break). But when
deploying a product it would be very bad to silently ignore a missing build file
because the finished product that was deployed might come out broken. I think
Dominique got it right, I just want the ability to loosen things up.

hehe mid-aired... 

hmm the fileset solution does sound like it would work. I suppose that pushes my
patch into  the realm of syntax sugar... I kinda like it though because it suits
how I think about the problem...
Comment 8 Gus Heck 2003-03-27 16:12:44 UTC
I was thinking about this on the drive to and from work and it seems that subant
class clearly was intended to support specifying a directory that contained a
build.xml as well as specifying the build file explicitly. There is some work
done to append the antfile to non-file path elements. Specifically this:

         for (int i=0; i<count; ++i) {
            File file = new File(filenames[i]);
            if (file.isDirectory()) {
                file = new File(file, antfile);
            }
            execute(file);
         }

Since the task clearly is meant to support the use of directories, I think it is
a good idea to give the users some control over how it handles missing build
files. Alternately, one might decide that it is clearer if the task only works
when the build file is specified and directories should always fail. I have a
build working, now and it could be conducted either with dirsets and my patch or
using Dominique's fileset method. It would be nice to know if I need to switch
to the fileset method or not.

My personal bias tends to be toward giving the user multiple options (so long as
it doesn't lead to confusion or really messy practices). If there are multiple
ways to do the same thing, a wider array of individuals will find that their
first instinct on how to acomplish something is correct. To me this is what
user-friendliness is all about :). I don't think my patch is large enough to
raise perfomance issues, so I am still hoping it gets applied. 
Comment 9 Gus Heck 2003-04-10 18:20:21 UTC
Any more thoughts on this? Dominique, what do you think, is this a reasonably
useful addition to subant? Any objections to someone commiting it? If not, any1
wanna do the commit? :)
Comment 10 Steve Loughran 2003-04-11 16:12:47 UTC
I'm still not enthused by it, given filesets do the right thing.
Comment 11 Gus Heck 2003-04-11 17:42:12 UTC
Maybe selecting by directories isn't what we want then? 

If one can specify directories, then the build breaks if someone adds a
directory that becomes included without also adding a working build file, or
adds an exclude to the build to handle the new directory. This brittleness may
be desireable, but if it is discovered accidentally, right now the only fix is
to recode the build file with a fileset that selects specific buildfile names
(making the antfile atribute useless). With this patch adding the
ignoreMissingBuildFiles atribute fixes the problem with no further rethinking of
the logic.

My concern is for people who do just what I did and assume that the way to use
subant is to tell it a antfile name and feed it directories. (why else would the
antfile atribute be there?)

This logic leads one down the brittle path first. If nothing else, I think this
issue might be good to mention in whatever manual docs or xdocs are provided for
subant in the future.

I suppose it could be that I stumbled into this minor hole simply because I
think in wierd ways...
Comment 12 Dominique Devienne 2003-04-11 18:39:17 UTC
Sorry Gus, I'll have to go with Steve on this one. I'm in favor of the changes 
for supporting an absolute antfile (not really the impl, more the use case), 
but your use case is already supported as-is, and I'm not too fond on your 
proposed changes personnally. Cheers, --DD
Comment 13 Gus Heck 2003-04-11 19:23:35 UTC
Hmm well I am open to suggestions as to how to improve my patch if there are
things you don't like about it. I'm not quite sure what you mean by "absolute
buildfile". 
Comment 14 Dominique Devienne 2003-04-11 19:30:55 UTC
I'm referring to http://nagoya.apache.org/bugzilla/show_bug.cgi?id=18715.
I was trying to say that I see that other use case as valid, and yours as 
invalid (but that's just me). --DD
Comment 15 Gus Heck 2003-04-12 03:37:56 UTC
Ah that bug slipped under my radar it would seem. I am perfectly happy if my
behavior is a useful part of a more interesting extension. I don't much care if
it is my patch or his. I also like his ideas.
Comment 16 Antoine Levy-Lambert 2003-05-21 07:36:11 UTC
I have committed the code for the bug 18715, which I have also closed for now.
Gus, if you have any issues, reopen 18715. Cheers.

*** This bug has been marked as a duplicate of 18715 ***
Comment 17 Gus Heck 2003-09-23 21:24:31 UTC
I've stubbed my toe on a use case that "just use filesets" does not cover now so
I am going to reopen this... I discovered it when I tried to migrate away from
my old patched version of ant (patched with the patch I submitted, and thus
allowing the ignoreMissingBuildFile atribute) to a newer version.

    <subant target="compile"
            buildpathref="path.dirs.copied.psets"
            ignoreMissingBuildFile="true">
      <property name="compile.failonerror" value="true"/>
    </subant>

Elswhere I define

    <path id="path.dirs.copied.psets">
      <dirset dir="${dir.build.src}">
        <include name="*/*/*/psets/*"/>
      </dirset>
    </path>


I definately want a dirset for this path, when it is used for copying as I want
the entire directories which include among other things, java code, libraries,
images and build files, but I definately don't want to fail if it copies a
directory that doesn't have a build file (there are many). I DO very much want
the build to fail if one of the sub builds fail. 

I would like to not need to maintain 2 specifications of this path. (the second
one being a fileset with the pattern "*/*/*/psets/*/build.xml). 

I am entirely open to an alternate (shorter!) name for the atribute, but don't
seem to be able to think of one I like better
Comment 18 Gus Heck 2003-09-24 16:21:03 UTC
Created attachment 8333 [details]
Since I had to re-add this for my build to work with 1.7alpha, here is the patch recreated vs the latest CVS