Add support for NFS backed config directories.

[deed02392]

The UID is impersonated when creating files, therefore, a check for correct permissions is done before attempting an operation that may fail e.g. a chown.

[ressu]

Can we adjust this to not parse human readable output:

if [ $(stat -c "%u:%g" ${APP_DATA}) != "${APP_UID:=0}:${APP_GID:=0}" ]; then

Also if we move the template copying below this test, we don't have to do assignment in all of the variables. (The :=0 bit..)

[deed02392]

From what I could find, using ls was the most portable solution to determining file permissions. stat has variable output across platforms whereas ls does not.

We have to make a decision on portability vs. slightly different looking code. Your decision.

[ressu]

That is a good point. At least busybox doesn't support format strings properly in stat, but considering that the image is built on Debian, we should be fine with using stat.

Also considering that we depend on external utilities anyway and we can't really run on a busybox environment, this should be a safe call in terms of portability as well.

[deed02392]

See new commit. We can't eliminate default value assignment from what I can see because it's possible that neither our conditions are met before finally attempting to launch python with gosu.

[ressu]

If we moved the permission check before the file copying, that way the assignment is triggered with the check. But your new solution works well too, so looks good to me.

[JackDandy]

When this PR is ready, can you squash/combine into 1 commit.
A new hotfix was pushed since this started so you will want to add that amendment before the squash.

Also, for the amended commit message, I suggest split it like so...

Add support for NFS backed config directories.

The UID is impersonated when creating files, therefore, a check for correct permissions is
done before attempting an operation that may fail e.g. a chown.

Line 1 is the short commit title, then a blank line, then a more descriptive comment to accompany the title line, you get the point ;)

[deed02392]

Can you advise on the best way to approach this with specific git commands?

…

On 13 July 2017 at 21:10, JackDandy ***@***.***> wrote: Can you squash/combine this PR into 1 commit, also, a new hotfix was pushed since this started so you will want to add that amendment before the squash. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#12 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADX01x0qL5Eud4kgPVQIk-nvyrtTRF-Nks5sNnmdgaJpZM4OR3ak> .

-- From George Hafiz

[ressu]

Rebasing needs a bit of practice before it feels natural, but the way it works is you take all the commits since the merge point and reorder or squash them into more clean commits. It can take a bit of time to get it right, but it helps keep the master repo clean.

This is one of the more comprehensive documentations on rebasing: https://www.atlassian.com/git/tutorials/rewriting-history#git-rebase-i

The TL;DR version is:

git rebase -i origin/development

(this is assuming that origin points to the upstream repo)

You get a list of commits and instructions. You squash the commits into logical blocks and edit the commit messages to make sense (this happens during the rebase process after you selected the actions)

Finally, when you are happy with the result you push the changes back to your branch. But since you have now altered the history, you need to force the push.

git push --force

[JackDandy]

I can squash it if you are both happy that the current state is complete.
@ressu if you can then set up/test the merge of what I output on develop to master, then we should be good.

[ressu]

Hmm.. this exec will actually prevent from rest of the script happening. exec command replaces the current script with the command about to be run. Simply removing the exec will fix the behavior

[JackDandy]

Done.

Note to self: do not use github "squash" as it will remove original author of a feature branch, use "merge squash" instead.

[JackDandy]

#13