git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] Make sure to use Araxis' "compare" and not e.g. ImageMagick's

From
Sebastian Schuberth <sschuberth@gmail.com>
Date
Jul 23, 2012, 21:09 UTC
Message-ID
<CAHGBnuOz94YR9wx_goL5YaWzPt5Z9c3gBB9CtyfcE40F5amrXw@mail.gmail.com>
In-Reply-To
<7v4noykxvm.fsf@alter.siamese.dyndns.org>
On Mon, Jul 23, 2012 at 10:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> For example, when the user tells it to install in "/home/ss/bin", if
> it installs its "compare" program in "/home/ss/bin/araxis/compare"
> without allowing the "/araxis/" part to be stripped away, the above
> heuristics is sufficiently safe.  Otherwise, it is not.

To the best of my knowledge, Araxis does not enforce any naming convention for the path it gets installed in. That means the user may indeed install the program in a path that does not contain "araxis". I was aware of this when writing the patch, but should have probably made it more clear in the commit message.

> It is unclear from your proposed commit log message what assurance
> do we have that it is installed under such a path and why the
> heuristics the patch implements is the sane way forward.

We have no such assurance. That's why you correctly call it a heuristics after all: it may fail. Personally, I've valued the gain of the patch to not list araxis as an available diff tool by "git difftool --tool-help" when in fact just ImageMagick is in PATH higher than the loss to support araxis installations that are in a path not containung "araxis" but are in PATH.

Please feel free to ignore the patch if you feel the heuristics is not sufficiently safe. I'm currently unable to come up with a safer solution while maintaining portability, i.e. not use "which" or doing rather laborious string parsing on the output of "type".

-- 
Sebastian Schuberth
Previous: Junio C HamanoNext: Junio C Hamano
Message 30 of 40 in “Various merge / diff tool related minor clean-ups and improvements”
  1. 0/4 Various merge / diff tool related minor clean-ups and improvementsSebastian Schuberth, Jul 23, 2012
  2. 0/5 Various merge / diff tool related minor clean-ups and improvementsSebastian Schuberth, Jul 23, 2012
  3. 1/5 Sort the list of tools that support both merging and diffing alphabeticallySebastian Schuberth, Jul 23, 2012
  4. 2/5 Use variables for the lists of tools that support merging / diffingSebastian Schuberth, Jul 23, 2012
  5. Junio C HamanoJul 23, 2012
  6. Sebastian SchuberthJul 23, 2012
  7. 1/4 Use variables for the lists of tools that support merging / diffingSebastian Schuberth, Jul 23, 2012
  8. Junio C HamanoJul 23, 2012
  9. Sebastian SchuberthJul 23, 2012
  10. 3/5 Explicitly list all valid diff tools and document --tool-help as an optionSebastian Schuberth, Jul 23, 2012
  11. Junio C HamanoJul 23, 2012
  12. mergetool: support --tool-help option like difftool doesJunio C Hamano, Jul 23, 2012
  13. Sebastian SchuberthJul 23, 2012
  14. mergetool: support --tool-help option like difftool doesSebastian Schuberth, Jul 23, 2012
  15. Junio C HamanoJul 23, 2012
  16. Sebastian SchuberthJul 23, 2012
  17. David AguilarJul 23, 2012
  18. Junio C HamanoJul 23, 2012
  19. Sebastian SchuberthJul 23, 2012
  20. Junio C HamanoJul 23, 2012
  21. Re*: mergetool: support --tool-help option like difftool doesJunio C Hamano, Aug 23, 2012
  22. David AguilarAug 23, 2012
  23. Junio C HamanoAug 23, 2012
  24. David AguilarAug 24, 2012
  25. Jens LehmannAug 26, 2012
  26. 4/5 Make sure to use Araxis' "compare" and not e.g. ImageMagick'sSebastian Schuberth, Jul 23, 2012
  27. Junio C HamanoJul 23, 2012
  28. Make sure to use Araxis' "compare" and not e.g. ImageMagick'sSebastian Schuberth, Jul 23, 2012
  29. Junio C HamanoJul 23, 2012
  30. Sebastian SchuberthJul 23, 2012
  31. Junio C HamanoJul 23, 2012
  32. Sebastian SchuberthJul 23, 2012
  33. David AguilarJul 23, 2012
  34. Sebastian SchuberthJul 23, 2012
  35. Andreas SchwabJul 23, 2012
  36. Junio C HamanoJul 23, 2012
  37. Sebastian SchuberthJul 23, 2012
  38. Junio C HamanoJul 23, 2012
  39. Sebastian SchuberthJul 23, 2012
  40. 5/5 Add a few more code comments and blank lines in guess_merge_toolSebastian Schuberth, Jul 23, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.