Sandstorm app package support

[ocdtrekkie]

With these changes, the Sandstorm packaged version can run the same code as a standard deployment with a single additional configuration option. I am not much of a JavaScript developer, so nitpicks are welcome.

Fixes #284

_{By opening a pull request, I certify that I hold the intellectual property of the code I am submitting,
and I am granting the initial authors of WBO a perpetual, worldwide, non-exclusive, royalty-free, and irrevocable license to this code.}

[lovasoa]

Thanks !

I don't have the competence to review and maintain the things in .sandstorm. I'll merge the changes to the application itself though, after you fix my little nitpicks :)

[lovasoa]

is this file auto-generated ? If so, then maybe it shouldn't be on git ?

[lovasoa]

do these file need to be on git ?

[lovasoa]

this does not work, does it ? uid and gid become local to the scope of the try/catch.

Looking at the code, we could avoid the call to userInfo completely in the normal case, since we use the uid/gid only when the writeFileSync fails.

  try {
    fs.writeFileSync(tmpfile, "{}");
    fs.unlinkSync(tmpfile);
  } catch (e) {
    let err_msg = "does not allow file creation and deletion. ";
    try {
        const { uid, gid } = os.userInfo();
        err_msg += "Check the permissions of the directory, and if needed change them so that " +
      `user with UID ${uid} has access to them. This can be achieved by running the command: chown ${uid}:${gid} on the directory`;
      } finally {
             return err_msg;
      }
  }

[lovasoa]

Suggested change

	/** If this variable is set, automatically redirect to this board. */
	/** If this variable is set, automatically redirect to this board from the root of the application. */

[lovasoa]

the double negation is hard to process. Maybe invert the if and else: if (config.DEFAULT_BOARD) ... else ...

[lovasoa]

The board name needs to be urlencoded.

[ocdtrekkie]

These changes worked for me in the sense that the Sandstorm package still works, and without the environment variable set, the index loads, but I suspect you are correct that I never actually test a path where the userInfo is actually used, and your solution is better. I will amend my changes tonight, along with the other changes.

As far as the contents of .sandstorm, generally, yes they all are committed together. pgp-signature is actually a file specific to this package which states I control the app publishing key for this package. While we discourage the Vagrantfile and global-setup.sh being edited, they carry some important information to replicate the build environment, such as what box is used, and regenerating them is only done in breaking scenarios, usually.

Honestly one of my unsolved problems is I did notice you don't commit your screenshots to the repo, and for the Sandstorm app metadata, we generally would. It is possible that upstreaming the package metadata is the wrong call if you prefer to keep a fairly lean tree: I potentially could PR the changes to WBO itself, but keep the .sandstorm directory on its own repository.

Instead of it being a fork of WBO, I can have the separate build repo clone the official WBO repo in the build process, which could be used verbatim since I'm upstreaming the needed changes to WBO to work on Sandstorm.

[lovasoa]

I potentially could PR the changes to WBO itself, but keep the .sandstorm directory on its own repository.
Instead of it being a fork of WBO, I can have the separate build repo clone the official WBO repo in the build process, which could be used verbatim since I'm upstreaming the needed changes to WBO to work on Sandstorm.

I think that would work better for me, yes !

[ocdtrekkie]

Awesome. I will do some reorganizing on my end and submit a PR with just the improved changes to your code.