command_exists is indeed called quite extensively, and
*must* ensure that a command is executable if it returns true.

This was the case when it called a shell command "which", but it
stopped being the case when for portability reasons this was replaced
with "type", which only checks whether a file with given name exists
in the path, and does not verify executable rights.

After being hit by this when testing with several external solvers, I took the
time to fix this error, namely:

  - use "command -v" whose semantics is properly specified in POSIX
    (see
    http://stackoverflow.com/questions/592620/how-to-check-if-a-program-exists-from-a-bash-script)

  - check that the user running opam has the right to execute
    the external command, by carefully verifying the execute bits

As for performance impact, once you decide to run an external shell,
the time spent calling Unix.stat is negligible.

On Fri, Apr 04, 2014 at 05:11:56AM -0700, Thomas Gazagnaire wrote:
> In src/core/opamSystem.ml:
> 
> >    OpamProcess.clean_files r;
> > -  OpamProcess.is_success r
> > +  if OpamProcess.is_success r then
> 
> Can you explain what you are doing here ? What are the performance impact ?
> command_exists is called a lot by opam, would be good to keep this as light as
> possible.
> 
> —
> Reply to this email directly or view it on GitHub.*
> 

-- 
Roberto Di Cosmo
 
------------------------------------------------------------------
Professeur               En delegation a l'INRIA
PPS                      E-mail: roberto@dicosmo.org
Universite Paris Diderot WWW  : http://www.dicosmo.org
Case 7014                Tel  : ++33-(0)1-57 27 92 20
5, Rue Thomas Mann       
F-75205 Paris Cedex 13   Identica: http://identi.ca/rdicosmo
FRANCE.                  Twitter: http://twitter.com/rdicosmo
------------------------------------------------------------------
Attachments:
MIME accepted, Word deprecated
      http://www.gnu.org/philosophy/no-word-attachments.html
------------------------------------------------------------------
Office location:
 
Bureau 3020 (3rd floor)
Batiment Sophie Germain
Avenue de France
Metro Bibliotheque Francois Mitterrand, ligne 14/RER C
-----------------------------------------------------------------
GPG fingerprint 2931 20CE 3A5A 5390 98EC 8BFC FCCA C3BE 39CB 12D3