Bug 22020 - [PATCH] addition of access atribute to <target..> task
Summary: [PATCH] addition of access atribute to <target..> task
Status: REOPENED
Alias: None
Product: Ant
Classification: Unclassified
Component: Core tasks (show other bugs)
Version: unspecified
Hardware: All All
: P3 enhancement with 1 vote (vote)
Target Milestone: ---
Assignee: Ant Notifications List
URL:
Keywords: PatchAvailable
: 3807 23397 28447 (view as bug list)
Depends on:
Blocks:
 
Reported: 2003-07-31 14:20 UTC by jesse farinacci
Modified: 2009-07-31 03:17 UTC (History)
5 users (show)



Attachments
Adds public/private access modifiers to <target> (4.79 KB, patch)
2003-08-11 19:16 UTC, Gus Heck
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jesse farinacci 2003-07-31 14:20:30 UTC
hello and good day--i apologize if the feature i am requesting has already been 
considered, to my knowledge it is not in existence or on the list of accepted / 
rejected items for consideration.  i would like to see a new optional flag be 
added to the <target..> task which allows it to be marked as internal use only.  
this would effectively keep out users from tasks which should not be invoked 
from the command line.  also, this would 'hide' the target, i.e. not be listed 
as a possible target task when -projecthelp is passed to ant.  thanks for such a 
great tool!!  :-)

-jlf
Comment 1 Gus Heck 2003-07-31 17:10:16 UTC
I implement something of the sort currently It works like this:

  <target name="public">
    <property name="invoked.public.target" value="true"/>
  </target>

  <target name="init">
     <fail unless="invoked.public.target">
     This target is not available to the general public.
     For a list of public targets use: ant targets
     </fail>
   ... rest of initialization ...
  </target>

  <target name="internal1" depends="init"/>
  <target name="internal2" depends="init"/>

  <target name="invokeMe" depends="public, init, internal, internal2"/>

   <target name="targets">
       <echo>
Public Targets for this ant file:
---------------------------------------------------------------------------
 invokeMe           Does incredibly useful stuff
---------------------------------------------------------------------------
       </echo>
   </target>

The key is to make every target depend on init and only available targets, the
major drawback is you have to maintain your own target to handle project help
type stuff. I would like to see this feature added as well, but I havn't taken
the time to come up with a patch because I already have the workaround in place :).
Comment 2 Gus Heck 2003-07-31 17:13:53 UTC
ack bitten by my own editing errors again. 

The above should say:

The key is to make every target depend on init and only available targets depend
on public, the major drawback...

Also, the depends for invokeMe should say internal1, internal2, but you probably
guessed that.
Comment 3 Steve Loughran 2003-07-31 18:39:07 UTC
1. give the target a name beginning with a hyphen '-' and it cannot be called on
the command line

2. give public entry points a description attribute and omit it from the others,
and when you do -projecthelp, the others dont get listed (except in -verbose mode)
Comment 4 Gus Heck 2003-08-04 20:36:02 UTC
When was this targets beginning with - thing introduced? Has it been there all
along? if so where is it documented I know I looked at one point for some form
of public/private target distinction and missed it (which isn't a very good test
at all, but there you have it) 

After testing it myself, I found that anything that begins with "-" is parsed as
an option, but what happens when things change? for example what if support for
"--" to prevent further option recognition is added? Sounds like a perfectly
reasonable feature if you don't know that people are using -internal1 to keep
targets hidden.

Also, the public/privateness is hidden in the text of the target name, rather
than out in an atribute of it's own where it can be easily changed or
programatically considered (perhaps in resolving imports? can an imported ant
file's "private" targets be called? What about overriding them?)

Steve's workaround is cool, clever and provides exactly what I want in terms of
hiding targets, but I am reopening because I think it also looks like a
coincidental result of option processing quirk and a more robust system that
doesn't push funtionality into the target names could be subsituted. This is
afterall only an enhancement request. (maybe after I move I'll try to write
this, if Verizon doesn't go on strike and deprive me of a phone line in my new
appartment) Others are of course welcome to beat me to it :)

I also noticed that this issue sort of came up in bug 3807 but the reporter
seems to have ignored the response given. Perhaps 3807 should be marked a dup of
this (since this one has more info on it already)
Comment 5 Steve Loughran 2003-08-04 21:43:38 UTC
Gus, you are 100% correct: any -something target is only private because of the
command line parsing process. It is a dirty trick, but it is also one we know
people use :)
Comment 6 Dominique Devienne 2003-08-04 22:25:37 UTC
Does that prevent us from adding a proper private="true/false" attribute to 
target, which would correctly enforce the target cannot be called using <ant> 
from the outside (not problem with leading - in the case), or cannot be called 
from a build file that imports the private target (in fact this private target 
would never be visible to the importing build file, which could merrily define 
a similarly named target... exactly like private methods work in Java BTW ;-).

Maybe it's time to get away from the leading dash - trick, and do something 
clean and documented??? --DD
Comment 7 Steve Loughran 2003-08-04 23:53:31 UTC
no, it doesnt at all...
Comment 8 Gus Heck 2003-08-11 19:15:11 UTC
Ok, I coded this up. I tried to write it in a way that access modifiers could be
used fairly generally by simply ehancing the isAccessibleFrom method that I
added to target. I have only implemented the from the modifiers public and
private, which will mean works on the command line and doesn't work on the
command line.

Those who would like to consider what should be done with import/overide etc can
simply add another access point name to the isAccessibleFrom method and then
check it in whatever code makes decisions about inclusion/invocation/precedence
of targets to find out what the author intended.

My implementation likely needs to have some constants added instead of the magic
words "command line". I wasn't sure where the best place to put such constants
would be, so I just left it. Edit or not as pleases (of course). A patch for
docs will be forthcomming, I want to see if people like my patch before I spend
time documenting. I suspect that there is no good way to write a unit test for
the invocation on the commandline, but if someone can tell me how I am willing
to learn :)

The following build file seems to work as expected... (which I used for testing)

<?xml version="1.0"?>
<project name="access-test" basedir="." default="pubtar">

<target name="pubtar" access="public">
  <echo message="default target called"/>
  <antcall target="privtar"/>
</target>

<target name="pubtar2" access="public" depends="privtar">
  <echo message="default target called"/>
  <antcall target="privtar"/>
</target>

<target name="privtar" access="private">
  <echo message="internal target called"/>
</target>

</project>
Comment 9 Gus Heck 2003-08-11 19:16:22 UTC
Created attachment 7756 [details]
Adds public/private access modifiers to <target>
Comment 10 Gus Heck 2003-08-18 16:20:32 UTC
Any1 have any thoughts on my patch here?
Comment 11 Jan Mat 2003-09-05 14:52:49 UTC
*** Bug 3807 has been marked as a duplicate of this bug. ***
Comment 12 Jan Mat 2003-09-25 07:52:37 UTC
*** Bug 23397 has been marked as a duplicate of this bug. ***
Comment 13 Gus Heck 2003-10-15 20:12:17 UTC
The comment in the middle of the post linked below also points to a user desire
for access control on targets, or in this case ways to work around needing it...
even if it isn't going to make it into the 1.6 can it go into 1.7? That way we
can start finding out why what I wrote is wrong (or not)?

http://marc.theaimsgroup.com/?l=ant-dev&m=106623431723810&w=2

Also to keep info available in one place, I would like to note that some other
relavant discussion of this RFE occured on the dev list here under the heading
of one of the duplicates:

http://marc.theaimsgroup.com/?l=ant-dev&w=2&r=1&s=Bug+23397&q=t
Comment 14 Matt Benson 2004-04-17 19:56:21 UTC
This seems to keep coming up and, like DD et al, I cannot think of a good 
reason to keep it out as long as we preserve BC regarding "-*" targets...
Comment 15 Matt Benson 2004-04-17 20:03:27 UTC
*** Bug 28447 has been marked as a duplicate of this bug. ***
Comment 16 Steve Loughran 2004-04-17 20:45:05 UTC
Well, now that the big complexity in the access modifiers exists -<import>, what
are we going to do with access. 

Which of these three do we want:
1. the access modifiers control access from the command line
2. private targets cannot be called from <ant> calls from other build files than
(self)
3. private targets cannot be called from imported files.

(3) scares me. Big time. add that and people will want protected, maybe even
package private. And it complicates importing no end. Would it be an error to
redefine a target that was private in an import? It is in C++ after all. Would
it be allowable to depend on a private target you import? 

I am personally biased towards (1) -we just need to make explicit that
public/private only applies to command line access, not to anything else. 

Nb, assuming we say "-*" defaults to private, what is the semantics of 
<target name="-secret" access="public"> ? 



Comment 17 Matt Benson 2004-04-17 21:10:12 UTC
I would agree that (1) is the simplest.  For clarification here and now, access 
defaults to "public".  I would treat the -* targets as a different issue; i.e. 
if a user can find a way to call a public (explicitly defined as such or 
otherwise) target named -* from the command line, they are welcome to do so.

As a long-overdue response to Gus's question(s) about the attached patch... I 
think an EnumeratedAttribute should be used for the access values... also, I 
don't know if the whole "access point" thing is warranted, especially if we 
choose that access is only relevant with respect to the command line.

$0.02
Comment 18 Matt Benson 2004-04-17 21:18:45 UTC
As an additional note, bug 24231 has historically been associated with this one 
and might be reexamined in terms of providing a way to implicitly allow -* 
tasks to be called from the command-line...
Comment 19 Gus Heck 2005-04-19 21:25:52 UTC
Well, 1.5 years later, I am again in need of something along these lines, so I
figured I might as well ping this bug...

The current scenario is I am trying to set up a simple text based interactive
build that uses input to collect various parameters (rather than having 15
different -D arguments). This will be distributed as part of a zip file and used
by the customer to create a warfile with the tweaks needed for their environment.

Since the customer gets the source code ayway, the quickest way to do this zip
up the source and write an small build file that imports the development build,
but hides dangerous targets from the user. It would be a disaster if they
accidentally ran recreate-db against their production DB since the first thing
it does is wipe the entire database.

This need is slightly different than the intent of my patch, but closely
related. I have created duplicate targets in the build file to match the one
imported which helps (forcing the targets to be refered to as
buildname.sometarget instead of just sometarget), but imported targets show up
under project help.

Has any further thought been given to controling access to targets? Have I
missed something in the past year? (I'm re-subscribing to the dev list so feel
free to answer me there)