Do we have a preference for #!/usr vs #! /usr? I don't really care, but just noticed that not all the scripts are consistent.

I've tended to prefer #! /usr, but don't feel strongly. I think we might need to do one more tiny cleanup pass before releasing to tighten up little details like this.

Style question, any reason not to one-line the locals?

In my old age, I've begun to favor "letting the code breathe" by splitting things across multiple lines. Which is to say, I used to optimize for vertical space in several languages, only to curse myself later for being so clever that I had to spend a disproportionate amount of time and mental energy unpacking my compact expressions when I had to revisit them later.

Granted, putting a couple of locals on the same line isn't a big deal, but I just split them to be consistent.

Yep. My admittedly narrow opinion is to distinguish "noise" from signal in the code. Variable declarations qualify as noise to me since they aren't really interesting, so I try to compress the amount of space that noise takes up, which in turn let's me more easily uncompress the meaningful parts of the code and let the signal breathe 😄

I'm not following how bats_absolute_path works to set BATS_ROOT. (And my macOS/bsd printf doesn't have -v documented... I think these things are related 😄 )

Ah, that's because printf is a Bash builtin, and the -v option takes the name of a variable into which to store the result. This is much quicker than capturing stdout into a variable via $().

Was that the only question you had about bats_absolute_path? Happy to unpack it some more.

Thanks! That was all I didn't quite grok. And got lazy trying to find docs on -v (even a cursory google search didn't include -v in their docs, so thanks for the lazyweb explain 😄 )

could $2 be assigned to a local as $1, or is there a reason it's not?

There is a reason: printf -v will write to a variable, and in this case, to a variable in the caller's scope, the name of which the caller specifies as $2. To avoid name collisions with variables outside of the scope of functions like this, I try to use positional parameters directly (whereas I normally do assign them to local variables for clarity's sake).

But I'm just realizing I've got two other local variables here, and I'm not following my usual practice of mangling their names to reduce the likelihood of collision (where "mangling" here is prefixing variable names with something like __bats_abs_path_ to reduce the likelihood of collision). I could also add such a local variable for $2 as well.

Any preference?

Thanks for the detail! Personal preference is to be explicit (I too forgot about printf's variable assignment), so a "mangled reference" (sounds like that should be a programming term!) would be my suggestion.

OK; I'll add those in #92. (Except for the one-liners, though.)

BTW, have you never done C++? "Name mangling" is a C++ implementation detail, but one that C++ coders need to be familiar with.

Only casual commits, never anything serious. Glad to have learned it though 🤣

So I pushed a new commit to #92, but only went so far, because the functions in libexec/bats-exec-test were mostly short, but all called frequently enough that there was a slight-yet-noticeable performance impact. I also decided not to mangle literally everything since these functions are all internal, so we control how they're called, and they're easier to read without all the mangling.

As above, also wonder if this should be refactored into a shared file as it's reused

I was thinking this through, and still am, but there's a chicken-and-egg problem—or there might not be, and I'd be interested in your thoughts.

Apologies in advance for the length.

The way the system works depends upon bin/bats discovering the real Bats root directory. For example, if Bats is installed at /opt/bats, then /opt/bats/bin/bats needs to find /opt/bats/libexec/bats. /opt/bats/libexec/bats then adds /opt/bats/libexec to the front of PATH, so then Bats can find the rest of the /opts/bats/libexec scripts. Because of this, /usr/local/bin/bats can symlink to /opt/bats/bin/bats, and the rest of Bats's internals can remain encapsulated within /opt/bats such that they don't pollute the /usr/local namespace (the man pages notwithstanding, unless /opt/bats/man is added to MANPATH somehow).

Previously, having bin/bats be a symlink to ../libexec/bats worked fine in general—but broke down in the /usr/local/bin/bats with /opt/bats scenario I described above, and was creating complications for some Windows-based platforms. Hence why I made bin/bats a normal script, then pushed the real path logic in there.

Now install.sh from sstephenson/bats also originally contained this real path-finding logic. Having duplicate code rubs me terribly wrongly, but if bin/bats needs to find its real path, and install.sh needs to find its real path...then how do they each find their real path so that they can import a shared script containing the real path logic?

One possibility I thought of, but I'm not sure it's worth the complication: If there were more than one Bats executable, they could all source ${0%/*}/bats, which could use ${BATS_SOURCE[1]} to discover which script sourced it, and could then dispatch logic between them. We could set up install.sh to source ${0%/*}/bin/bats to achieve this—but then we presume install.sh isn't symlinked directly, which would land us back in the same predicament.

But what if we assumed (or specified that) install.sh itself won't (shouldn't) ever be symlinked? In that case we probably could just compute the Bats root directory as the parent of install.sh (${0%/*}) without finding the real path. In other words, we can't assume /usr/local is the correct Bats root for /usr/local/bin/bats, but we may be safe in assuming (or specifying) that /opt/bats is the correct Bats root for /opt/bats/install.sh. This would eliminate the need for the duplicated code in the latter script.

Thoughts?

OK, I've convinced myself install.sh won't (shouldn't) be run from a symlink, so I've removed the duplicate real path finding code in a new commit on #92. Let me know if that's any cause for concern.

Hehe, I've been mulling it over and hacking around. Will pull the branch and check it out, cheers.

Sorry to cause more problems (!) - in writing the usage guide in #94 I found an edge case, which I've put into the travis build as per #93. Essentially the changes in #92 break running the tests when the tests are mounted into the container at a path that isn't a direct decedent of the bats base directory.

Current master doesn't fail under these conditions. I don't suspect anybody except us is using the pattern described above, but I can't be sure of the repercussions as I'm not as close to this code as you are,

Since it's the unofficial Bats strict mode test that's failing, I think I need to add a couple declare statements back into install.sh and that should do the trick. Will try in a couple hours.

Wait, that might not be it...either way, sure it's something small, and I'm doing a terrible job multitasking right now. :-P

To close the loop on this: I found and explained the breakage in #93, which actually wasn't due to #92. The "unofficial Bash strict mode" test failed because it was looking for bats relative to BATS_TEST_DIRNAME, which it couldn't because the test directory was mounted into the container in isolation. Mounting the root of the Bats project resolved the issue.