Fix ShellCheck errors + Add support for generic shell script

[k-lar]

Sometimes I have projects that don't require tooling systems to be present in the repository but I do need a build step, that's where a build.sh comes in. I've seen this in other people's projects too so this would be a nice feature to merge.

I also took care of some low hanging fruit by fixing errors thrown by ShellCheck, thereby achieving POSIX compliance.

Makefile was also expanded to include install and uninstall targets to make it easier to do those things.

All tests pass and I also added one for build.sh.

I can also add support for .bat and .ps1 scripts.

There could also be a tests.sh or something like that that could be supported to have a generic way to build/run/test smaller projects.

[paldepind]

Hello @k-lar. Thanks a lot for the PR – lots of great stuff in there.

I'm not sure what I think about supporting build.sh files though. There's two reasons why I'm a bit hesitant:

1. It's quite easy to add a simple Makefile for running such script. Take projectdo itself as an example. The tests are a simple script run-tests.sh which projectdo doesn't know how to run. To fix that all I had to do was a a makefile with the content:

   test:
   	sh run-tests.sh
   

   A similar thing could be done to run a build.sh script. A benefit to this approach is that it also documents how to build/test through the Makefile, it scales to both testing and building, and it works no matter what the script is named.

2. All the existing ways of building that projectdo supports amount to finding a "build command" of some sort. Running a shell script by doesn't fit into this pattern, and as such seems to expand the scope of projectdo a bit.

That being said I'm not entirely opposed to this either. A good argument in favor is that it is quite unlikely that a present build.sh script is not going to build the project.

I would love to hear what you think about the above and thanks again for the PR :)

[k-lar]

Thanks @paldepind for taking a look!

I do agree that it's trivial to add the build commands to a Makefile, but I did this from the perspective of allowing people to minimize the use of external tools and to enable a "shell-centric" approach that at least I enjoy.

1. Of course Makefiles are better, but they are external and aren't guaranteed to be installed
   (happened to me a couple times and I was without internet so I couldn't download the package)
   This feature would allow you to have a quick and dirty build step if you don't want to deal with downloading/learning make, and the added benefit of being able to build projects that already have a build.sh file :)

2. The scope could be limited by specifying that only build.sh files are supported and that it's not guaranteed to work because we can't reliably detect build tools inside the script. I guess we could annoy the user with a warning that explains that every time it runs a build.sh and they can disable it with a flag? I think that should be written only in the README / manpage when it gets made but the flag approach also works.

I love the idea of this project and I might contribute more! I can definitely make a manpage for it in a couple of days and I can probably conjure up some other stuff that might be useful.

[paldepind]

Could this be ./build.sh? That could work better if the script is a Bash script and the user is running the Bash shell.

[k-lar]

Oh I didn't know that, sure I'll change it to ./build.sh.

[paldepind]

In general sh script.sh will execute sh and have it run script.sh. When doing ./script.sh the shebang in the top of the script determines what program the script is run through. This makes a difference if the shebang does not specify sh. For instance if the shebang is #!/bin/bash, then the script should be run with bash. In theory the script could also be written in Python or JavaScript or something else (though it wouldn't make sense with the .sh extension).

What I wrote before was slightly inaccurate, the users shell doesn't matter. Running a Bash specific script with ./script.sh works no matter what the users shell is.

[paldepind]

I think this else branch could be removed. It will echo and exit every time projectdo gets to to try_build_script and we will never get to the generic nothing_found function.

[paldepind]

Thanks for the reply. I think those are good points. I definitely see the argument that it's nice to support a way of building that is completely dependency-free which the tool based ones obviously can't be.

The documentation currently suggests doing a dry-run if running projectdo in a new project where one is not exactly sure what it will do. Given that, I think it's fine to just run the build.sh script. If that ever becomes a problem then we can consider the option with a warning or a flag. It would also be a possibility to use an environment variable to configure the behavior. But again, I think it's fine but its good to just keep those option in mind.

I love the idea of this project and I might contribute more! I can definitely make a manpage for it in a couple of days and I can probably conjure up some other stuff that might be useful.

I'm happy to hear that you like the idea 😄. A manpage would be a really great addition!

[paldepind]

I think this will pass all the arguments along to the tool as one single argument with spaces. We want to actually pass the arguments along to the tool as multiple arguments.

I think we need to preserve the TOOL_ARGS=$@ below. I found this SO answer which seems imply that $@. We need to keep the echo line or find something equivalent. Shellcheck complains about the eval but maybe that is a false positive or maybe there is a better way of achieving what the original eval line does in a better way?

[k-lar]

Okay, I reverted it back to the previous state but we should probably look into that a bit more because judging from the information here, the behavior of string=$@ is dependent on the shell and using $* would eliminate that uncertainty.

[paldepind]

That is a good idea. I think I'll see if I can add a test that ensures that the arguments are passed correctly, and then from there we can refactor while ensuring that the behavior is preserved :)

[k-lar]

I think it would also be helpful to integrate ShellCheck into the testing somehow, either that or to integrate it into CI for it to catch errors and if there is something that we want to ignore, we would have to explicitly disable warnings for that specific code.

[paldepind]

Thanks a lot for the PR!