{"action":"created","issue":{"url":"https://api.github.com/repos/bcpierce00/unison/issues/36","repository_url":"https://api.github.com/repos/bcpierce00/unison","labels_url":"https://api.github.com/repos/bcpierce00/unison/issues/36/labels{/name}","comments_url":"https://api.github.com/repos/bcpierce00/unison/issues/36/comments","events_url":"https://api.github.com/repos/bcpierce00/unison/issues/36/events","html_url":"https://github.com/bcpierce00/unison/pull/36","id":174435122,"number":36,"title":"Fixed tags Makefile recipe","user":{"login":"rjlasko","id":6244698,"avatar_url":"https://avatars.githubusercontent.com/u/6244698?v=3","gravatar_id":"","url":"https://api.github.com/users/rjlasko","html_url":"https://github.com/rjlasko","followers_url":"https://api.github.com/users/rjlasko/followers","following_url":"https://api.github.com/users/rjlasko/following{/other_user}","gists_url":"https://api.github.com/users/rjlasko/gists{/gist_id}","starred_url":"https://api.github.com/users/rjlasko/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rjlasko/subscriptions","organizations_url":"https://api.github.com/users/rjlasko/orgs","repos_url":"https://api.github.com/users/rjlasko/repos","events_url":"https://api.github.com/users/rjlasko/events{/privacy}","received_events_url":"https://api.github.com/users/rjlasko/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-09-01T04:39:52Z","updated_at":"2016-09-12T06:26:31Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/bcpierce00/unison/pulls/36","html_url":"https://github.com/bcpierce00/unison/pull/36","diff_url":"https://github.com/bcpierce00/unison/pull/36.diff","patch_url":"https://github.com/bcpierce00/unison/pull/36.patch"},"body":"Hello.\r\n\r\nI **love** Unison, and have been using it for the past 3 years.  Since I like to be able to control my software build process + version, I build Unison from scratch whenever possible.  To date, I have built Unison on many different platforms (OSx, FreeBSD, Debian, Ubuntu, Arch Linux, Synology Linux).  However, in every build (except OSx, where the 'etags' executable exists), I have encountered the same (non-fatal) error at the very end of the build process:\r\n\r\n```\r\nmake tags\r\nmake[1]: Entering directory '/tmp/src/unison-2.48.3'\r\nif [ -f `which etags` ]; then \\\r\n    etags *.mli */*.mli *.ml */*.ml */*.m *.c */*.c *.txt \\\r\n          ; fi \r\n/bin/sh: 2: etags: not found\r\nMakefile:347: recipe for target 'tags' failed\r\nmake[1]: [tags] Error 127 (ignored)\r\nmake[1]: Leaving directory '/tmp/src/unison-2.48.3'\r\n```\r\n\r\nDespite the build completing successfully, and everything running fine, this error has always bothered me, as the code indicates that the 'etags' executable is not necessary to complete the build of the unison executable.  I've tracked down the error to [this line of code](https://github.com/bcpierce00/unison/blob/master/src/Makefile#L341) in the source Makefile.\r\n\r\nFirst, the 'which' command (which is not as portable as 'command' or 'type') will return nothing if the file is not found, which means that '[ -f \"\" ]' will be the command run, which is both valid and evaluates true.  This combination is what yields the error above.\r\n\r\nTo fix the problem, we can instead interpret the (which, command, or type) executable generating some output to stdout as an indication that the directive exists or executable file is present on the PATH.  Our saviour here is the syntax '[ -n \"/x/y/z\" ]', which tests if the string is **not** empty.  In this case, '[ -n \"/some/path/etags\" ]' evaluates to true, and we know the command exists and we can proceed to run the etags command.  If not, then we instead get '[ -n \"\" ]', which evaluates false, allowing us to skip.\r\n\r\nI've tested this on Ubuntu 16.04 with BASH, and it works fine.  Easy peasy.  Hopefully you will accept this pull request, as it is pretty straightforward.\r\n\r\nThanks,\r\nRob\r\n"},"comment":{"url":"https://api.github.com/repos/bcpierce00/unison/issues/comments/246259628","html_url":"https://github.com/bcpierce00/unison/pull/36#issuecomment-246259628","issue_url":"https://api.github.com/repos/bcpierce00/unison/issues/36","id":246259628,"user":{"login":"ian-kelling","id":54232,"avatar_url":"https://avatars.githubusercontent.com/u/54232?v=3","gravatar_id":"","url":"https://api.github.com/users/ian-kelling","html_url":"https://github.com/ian-kelling","followers_url":"https://api.github.com/users/ian-kelling/followers","following_url":"https://api.github.com/users/ian-kelling/following{/other_user}","gists_url":"https://api.github.com/users/ian-kelling/gists{/gist_id}","starred_url":"https://api.github.com/users/ian-kelling/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ian-kelling/subscriptions","organizations_url":"https://api.github.com/users/ian-kelling/orgs","repos_url":"https://api.github.com/users/ian-kelling/repos","events_url":"https://api.github.com/users/ian-kelling/events{/privacy}","received_events_url":"https://api.github.com/users/ian-kelling/received_events","type":"User","site_admin":false},"created_at":"2016-09-12T06:26:31Z","updated_at":"2016-09-12T06:26:31Z","body":"I actually changed that line of code recently to deal with this issue. I only tested the it on debian, but it's working there.\r\n\r\n>  '[ -f \"\" ]' will be the command run, which is both valid and evaluates true\r\n\r\nNo, that evaluates to  false under dash and bash. The old code incorrectly evaluated to true. Try out the latest sources and let us know if it is still a problem."}}
{"actor":{"display_login":"ian-kelling"}}