These still seem to have the ${PATH}
e.g.:
34 PATH=/usr/bin:/usr/sbin:/sbin:/usr/lib/installadm:${PATH}; export PATH
Joe
John Fischer wrote:
> Joe,
>
> Opps... It should be:
>
> http://cr.opensolaris.org/~johnfisc/path-7325/
>
> Thanks,
>
> John
>
> Joseph J VLcek wrote:
>> John Fischer wrote:
>>> Joe,
>>>
>>> Updated webrev at:
>>>
>>> http://cr.opensolaris.org/~johnf/path-7325
>>
>> John,
>>
>> The webrev is not there.
>>
>> Joe
>>
>>>
>>> This time without the installadm_util.c change.
>>>
>>> Thanks,
>>>
>>> John
>>>
>>> John Fischer wrote:
>>>> Joe,
>>>>
>>>> I'll drop it then.
>>>>
>>>> John
>>>>
>>>> Joseph J. VLcek wrote:
>>>>> Thanks John!
>>>>>
>>>>>
>>>>> My comments below.
>>>>>
>>>>> Joe
>>>>>
>>>>> John Fischer wrote:
>>>>>> Joe,
>>>>>>
>>>>>> Thanks!!
>>>>>>
>>>>>> John
>>>>>>
>>>>>> Joseph J. VLcek wrote:
>>>>>>> Hey John,
>>>>>>>
>>>>>>> usr/src/cmd/installadm/installadm_util.c
>>>>>>> Still have the PATH setting logic. Can't this go away now?
>>>>>>
>>>>>> Yes. But... I wanted to be certain that should the function
>>>>>> (installadm_system()) be used in another way (i.e., not to fork
>>>>>> one of the scripts) that we got the correct PATH in that case
>>>>>> as well. Am I being overly cautious?
>>>>>
>>>>> Hum... I see your logic but I think it is not necessary.
>>>>>
>>>>>>
>>>>>>>
>>>>>>> All the scripts have:
>>>>>>> PATH=/usr/bin:/usr/sbin:/sbin:/usr/lib/installadm:${PATH}; export
>>>>>>> PATH
>>>>>>>
>>>>>>> I don't think you what the ${PATH}.
>>>>>>>
>>>>>>> I think it should be:
>>>>>>>
>>>>>>> #
>>>>>>> # Establish PATH for non-built in commands
>>>>>>> #
>>>>>>> export PATH=/usr/bin:/usr/sbin:/sbin:/usr/lib/installadm
>>>>>>>
>>>>>>
>>>>>> I will drop the $PATH.
>>>>>>
>>>>>>> Joe
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> John Fischer wrote:
>>>>>>>> Joe,
>>>>>>>>
>>>>>>>> Thanks for your help and patience.
>>>>>>>>
>>>>>>>> I have finally updated the webrev for this bug. I now have the
>>>>>>>> scripts setting the PATH appropriately. The scripts now contain
>>>>>>>> the line:
>>>>>>>>
>>>>>>>> PATH=/usr/bin:/usr/sbin:/sbin:/usr/lib/installadm:${PATH};
>>>>>>>> export PATH
>>>>>>>>
>>>>>>>> The new webrev is located at:
>>>>>>>>
>>>>>>>> http://cr.opensolaris.org/~johnfisc/path-7325/
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> John
>>>>>>>>
>>>>>>>> Joseph J. VLcek wrote:
>>>>>>>>> John Fischer wrote:
>>>>>>>>>> Joe,
>>>>>>>>>>
>>>>>>>>>> Thanks for taking the time to review the webrev.
>>>>>>>>>>
>>>>>>>>>> It is fairly simple to add the path this way. I think that the
>>>>>>>>>> best thing to do is have a standard of how the underlying scripts
>>>>>>>>>> are written. I am bent towards always full qualifying every
>>>>>>>>>> command from my security days in the desktop.
>>>>>>>>>>
>>>>>>>>>> If the scripts will be used directly from the command line or if
>>>>>>>>>> another fork/exec mechanism is developed within the installadm
>>>>>>>>>> command
>>>>>>>>>> then you are absolutely correct in saying that there will be
>>>>>>>>>> additional
>>>>>>>>>> problems.
>>>>>>>>>>
>>>>>>>>>> Looking at the scripts I see the following:
>>>>>>>>>>
>>>>>>>>>> 1) VARIABLE=<some binary>
>>>>>>>>>> ...
>>>>>>>>>> $VARIABLE ...
>>>>>>>>>>
>>>>>>>>>> i.e., GREP=/usr/bin/grep
>>>>>>>>>> $GREP "something" /etc/hosts
>>>>>>>>>>
>>>>>>>>>> 2) <some binary> # No PATH set
>>>>>>>>>>
>>>>>>>>>> i.e., grep "something" /etc/hosts
>>>>>>>>>>
>>>>>>>>>> 3) <some binary> # PATH set
>>>>>>>>>>
>>>>>>>>>> i.e., PATH=/usr/bin:/usr/sbin:/sbin:$PATH
>>>>>>>>>> grep "something" /etc/hosts
>>>>>>>>>>
>>>>>>>>>> 4) <fully path-ed binary>
>>>>>>>>>>
>>>>>>>>>> i.e., /usr/bin/grep "something" /etc/hosts
>>>>>>>>>>
>>>>>>>>>> The problem is that I see all 4 of these being used within the
>>>>>>>>>> scripts (actually sometimes all 4 in the same script) which is
>>>>>>>>>> probably from different developer's styles.
>>>>>>>>>>
>>>>>>>>>> Adding the PATH to all the scripts is also a simple solution and
>>>>>>>>>> could become the standard way we write scripts within the
>>>>>>>>>> component.
>>>>>>>>>>
>>>>>>>>>> Are you recommending that I modify the solution to add PATH to
>>>>>>>>>> each of the scripts?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> John
>>>>>>>>>>
>>>>>>>>>> Joseph J. VLcek wrote:
>>>>>>>>>>> John Fischer wrote:
>>>>>>>>>>>> All,
>>>>>>>>>>>>
>>>>>>>>>>>> Fairly straight forward fix to a PATH issue. Low hanging
>>>>>>>>>>>> fruit for the
>>>>>>>>>>>> new guy. More exciting things to come next week. ;-)
>>>>>>>>>>>>
>>>>>>>>>>>> webrev:
>>>>>>>>>>>>
>>>>>>>>>>>> http://cr.opensolaris.org/~johnfisc/path-7325/
>>>>>>>>>>>>
>>>>>>>>>>>> Open Solaris defect:
>>>>>>>>>>>>
>>>>>>>>>>>> 7325 - installadm script should set PATH
>>>>>>>>>>>>
>>>>>>>>>>>> The fix is to use putenv() of a sensible path which is
>>>>>>>>>>>> /usr/bin,
>>>>>>>>>>>> /usr/sbin and /usr/lib/installadm. Only set once for the
>>>>>>>>>>>> process
>>>>>>>>>>>> via an initialize variable.
>>>>>>>>>>>>
>>>>>>>>>>>> When someone has a chance I would appreciate a review.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>
>>>>>>>>>>>> John
>>>>>>>>>>>> _______________________________________________
>>>>>>>>>>>> caiman-discuss mailing list
>>>>>>>>>>>> caiman-discuss at opensolaris.org
>>>>>>>>>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>>>>>>>>>> John,
>>>>>>>>>>>
>>>>>>>>>>> I think having the installadm_system() function in
>>>>>>>>>>> usr/src/cmd/installadm/installadm_util.c export a PATH is an
>>>>>>>>>>> simple and elegant solution. However I think it might be
>>>>>>>>>>> safer in the long run to have each script define PATH.
>>>>>>>>>>>
>>>>>>>>>>> This would make each script more robust and able to stand on
>>>>>>>>>>> it's own without the requirement it be invoked by
>>>>>>>>>>> installadm_system().
>>>>>>>>>>>
>>>>>>>>>>> My concern is what happens if in the future a code change is
>>>>>>>>>>> made to have the scripts no longer invoked by
>>>>>>>>>>> installadm_system().
>>>>>>>>>>>
>>>>>>>>>>> What do you think?
>>>>>>>>>>>
>>>>>>>>>>> Joe
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> installadm_system
>>>>>>>>>>>
>>>>>>>>> Hey John,
>>>>>>>>>
>>>>>>>>> Yes I would suggest adding PATH to each script.
>>>>>>>>>
>>>>>>>>> There is already a standard but as you point out it hasn't
>>>>>>>>> alway been followed.
>>>>>>>>>
>>>>>>>>> The standard can be seen here:
>>>>>>>>> http://hub.opensolaris.org/bin/view/Community+Group+on/shellstyle
>>>>>>>>>
>>>>>>>>> Which describes: There are two acceptable ways to do this:
>>>>>>>>> (1) make *all* command references in your script use explicit
>>>>>>>>> pathnames or (2) explicitly reset $PATH in your script.
>>>>>>>>>
>>>>>>>>> This is also documented here:
>>>>>>>>> http://installzone-wiki.central.sun.com/wiki/index.php/Ksh93_Tips
>>>>>>>>>
>>>>>>>>> and here:
>>>>>>>>> http://hub.opensolaris.org/bin/view/Project+shell/shellstyle#always_set_path
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> As you point out, it is true that our scripts don't always
>>>>>>>>> follow this and sometimes mix methods. However I think when
>>>>>>>>> possible we should try to have our code adhere to this method.
>>>>>>>>> Which would lead us to have PATH defined in each script or
>>>>>>>>> alway use full path names.
>>>>>>>>>
>>>>>>>>> Some scripts indirectly use full path names with the CMD=<full
>>>>>>>>> path>; $CMD method.
>>>>>>>>>
>>>>>>>>> Joe
>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>> _______________________________________________
>>>> caiman-discuss mailing list
>>>> caiman-discuss at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>
