Revamps block logic to track full blocks

[TekWizely]

TBD

---

@flaix I wanted to get this PR up before crashing for the nite - It should fix the bad logic introduced in last version - I'll update you with more deets soon ...

[TekWizely]

@flaix,

As part of creating this PR, I took your shared Dockerfile template and created two different versions of it:

• Dockerfile2.template.txt
• Dockerfile3.template.txt

Each version adds extra formatting to make a cleaner separation between the templating and the output text.

The first file (v2) keeps the % # comment technique, which I quite like btw.

The second file (v3) moves the comments into % statement blocks, joining adjacent blocks when available.

I've confirmed that each file produces the exact same rendered content as your original file. You can diff -w the generated scripts to see that the the only difference between them is leading whitespace.

This (hopefully) shows off bash-tpl's indentation-tracking abilities.

Have a look and lemme know your thoughts !

-TW

[flaix]

@TekWizely,

I am happy to see, and to report, that my original comment is unfounded now, i.e. fixed, and that the output of the generated script is as it was before. That is to say, that the indentation in the manually added script part is keeping the intended indentation.

I also learned this neat trick, that indenting before the % will indent the statement line, which is something I will make use of. Meaning, that instead of using my C-style preprocessor inspired way of writing

% if [[ "$IMAGE_TYPE" == release ]] ; then
ENV GITBLIT_DOWNLOAD_SHA <% ${GITBLIT_DOWNLOAD_SHA} %>
ENV GITBLIT_DOWNLOAD_URL https://github.com/gitblit/gitblit/releases/download/v${GITBLIT_VERSION}/gitblit-${GITBLIT_VERSION}.tar.gz

%   if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
# Install su-exec to step down from root
RUN set -eux; \

I get a better result with

% if [[ "$IMAGE_TYPE" == release ]] ; then
ENV GITBLIT_DOWNLOAD_SHA <% ${GITBLIT_DOWNLOAD_SHA} %>
ENV GITBLIT_DOWNLOAD_URL https://github.com/gitblit/gitblit/releases/download/v${GITBLIT_VERSION}/gitblit-${GITBLIT_VERSION}.tar.gz

  % if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
# Install su-exec to step down from root
RUN set -eux; \

as it will render in the generator script as

if [[ "$IMAGE_TYPE" == release ]] ; then
printf "%s\n" ENV\ GITBLIT_DOWNLOAD_SHA\ "${GITBLIT_DOWNLOAD_SHA}"
printf "%s\n" ENV\ GITBLIT_DOWNLOAD_URL\ https://github.com/gitblit/gitblit/releases/download/v\$\{GITBLIT_VERSION\}/gitblit-\$\{GITBLIT_VERSION\}.tar.gz
printf "\n"
  if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
printf "%s\n" \#\ Install\ su-exec\ to\ step\ down\ from\ root
printf "%s\n" RUN\ set\ -eux\;\ \\

which I like. I actually liked my original way of writing it better, because that makes it easier in the template file to tell apart template and bash-tpl statement lines (because the latter all start with % at the beginning of the line and one doesn't have to search for it further down the line), but I also like the indentation in the generator script.

As for the original idea to be able to add template lines directly into statement block, I can't really comment on it since I have never felt the use for that. I do have another comment on indenting which I will put into a second comment to keep the matters apart.

👍

[flaix]

I see and understand that the whole indentation subject is rather complicated. I understand why you added the extra whitespace in the template, to make things more readable. It is something I would probably not use. Maybe because I am too used to old ways of templating or mixing two languages/systems in one file. But here is a reason why I would lean to not doing that and why I am rather concerned seeing it.

To me what is most important is the end end result, i.e. the file that the template script generates. Because that is what will actually be used in whatever use case. Well, not true, I am using bash-tpl in exactly one case and in that case, as any other I can see me using it, the end result is most important. (I could imagine cases where the end result is not important because it is just some HTML file nobody looks at.)

What is also important is the template file because that is the one getting edited and worked on. So a good structure is important like for any source code. Here is where your way of indenting comes in, I guess. Thirdly, I pay attention to the scripting part in the template script, i.e. the stuff I put in there to make it a script doing more than just what is automatically generated. Because that is something that an advanced user who makes use of that script may read to understand what is going on and maybe even alter, fix or extend.

I do not care too much about the automatically generated script parts. Also because, honestly, the print statements are all over the place anyhow. I mean, this is not really how I would write or read a bash script file:

if [[ "$IMAGE_TYPE" == release ]] ; then
printf "%s\n" ENV\ GITBLIT_DOWNLOAD_SHA\ "${GITBLIT_DOWNLOAD_SHA}"
printf "%s\n" ENV\ GITBLIT_DOWNLOAD_URL\ https://github.com/gitblit/gitblit/releases/download/v\$\{GITBLIT_VERSION\}/gitblit-\$\{GITBLIT_VERSION\}.tar.gz
printf "\n"
  if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
printf "%s\n" \#\ Install\ su-exec\ to\ step\ down\ from\ root
printf "%s\n" RUN\ set\ -eux\;\ \\
    printf "%s\n" \ \ \ \ apk\ add\ --no-cache\ \\
        printf "%s\n" \ \ \ \ \ \ \ \ \'su-exec\>=0.2\'\ \\
        printf "%s\n" \ \ \ \ \ \ \ \ \;\ \\
    printf "%s\n" \ \ \ \ \\
    printf "%s\n" \ \ \ \ \\
printf "%s\n" \#\ Download\ and\ install\ Gitblit

Now, with that being said, here is what I wanted to comment on. Just because I noticed it in your changes to my template, not because it pertains to the PR at hand.

As I said, I understand the idea behind

% if [[ "$IMAGE_TYPE" == release && "$DOCKERFILE_TYPE" == alpine ]] ; then
    FROM openjdk:8-jre-alpine

    # add our user and group first to make sure their IDs get assigned consistently, regardless of whatever packages get added
    RUN addgroup -S -g 8117 gitblit && adduser -S -H -G gitblit -u 8117 -h /opt/gitblit gitblit
% elif [[ "$IMAGE_TYPE" == release && "$DOCKERFILE_TYPE" == ubuntu ]] ; then

instead of

% if [[ "$IMAGE_TYPE" == release && "$DOCKERFILE_TYPE" == alpine ]] ; then
FROM openjdk:8-jre-alpine

# add our user and group first to make sure their IDs get assigned consistently, regardless of whatever packages get added
RUN addgroup -S -g 8117 gitblit && adduser -S -H -G gitblit -u 8117 -h /opt/gitblit gitblit
% elif [[ "$IMAGE_TYPE" == release && "$DOCKERFILE_TYPE" == ubuntu ]] ; then

I would chalk that up under personal preference. I may be too used to the template text appearing in exactly the way it should appear in the output, than having it interact with template language statements in terms of indentation. What concerns me is how this would be able to handle the case where indentation of the template text in-between statement lines is exactly as it should be, but is altered in the output because the templating tries to be too smart.

Take this example I added to my template for demonstration for what I mean:

# This is just a test
# Install gosu to step down from root
RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
% if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
        wget \
% fi
        gosu \
        ; \
    \

My expected end result in the truecase is, of course:

# This is just a test
# Install gosu to step down from root
RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
        wget \
        gosu \
        ; \
    \

But what I get is:

# This is just a test
# Install gosu to step down from root
RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
wget \
        gosu \
        ; \
    \

As you can see this affects the file that I care about the most, the end result. It is not an issue with the latest changes, though, as it was this way before already. Actually, this is where the template line within a statement block can come to rescue now, as it gets the desired result. Because the possible work-around before, i.e.:

RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
% if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
% echo "        wget \\"
% fi
        gosu \
        ; \

did also not generate a correct result:

RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
        wget \
gosu \
; \

Anyways, this is just to say that in my personal opinion indentation in the end result and in manually, intentionally added scripting in the template script is more important than getting the auto-generated lines printing the template text right in the template script. If I wanted to get the latter, I would probably employ some logic trying to detect bash script blocks and adding it according to that, independent from what was in the template.

Please take this only as my thoughts, as with the template-in-statement-block there would now be a solution for the problem mentioned above.

[TekWizely]

Thanks for the long replies - I will read and process all of it over the next day or ...

Quick chime in: re:

# This is just a test
# Install gosu to step down from root
RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
% if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
        wget \
% fi
        gosu \
        ; \
    \

The way that bash-tpl encourages you to think about the template formatting is:

# This is just a test
# Install gosu to step down from root
RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
        % if [[ "$DOCKERFILE_TYPE" == alpine ]] ; then
            wget \
        % fi
        gosu \
        ; \
    \

The % starts at the indention level of text you're generating. The wget is further indented to keep the template easier to read, but is magically unindented for the rendered template.

This gives the output you're hoping for:

$ DOCKERFILE_TYPE=alpine source <( bash-tpl test.tpl )

# This is just a test
# Install gosu to step down from root
RUN set -eux ; \
    apt-get update && apt-get install -y --no-install-recommends \
        wget \
        gosu \
        ; \
    \

[TekWizely]

I also learned this neat trick, that indenting before the % will indent the statement line

YES ! thats part of bash-tpl's smart logic at play - The indentation of the % dictates the indentation of the rendered script statements, as well as embedded text elements when used as a 'block'

Another example of it:

before text
        before text indented
        % if true; then
                test 1 before text
                % if true; then
                        double true !
                % fi
                test 1 after text
        % fi
        after text indented
after text

Due to indentation tracking/fixing, you get:

generated script

printf "%s\n" before\ text
        printf "%s\n" \ \ \ \ \ \ \ \ before\ text\ indented
        if true; then
                printf "%s\n" \ \ \ \ \ \ \ \ test\ 1\ before\ text
                if true; then
                        printf "%s\n" \ \ \ \ \ \ \ \ double\ true\ \!
                fi
                printf "%s\n" \ \ \ \ \ \ \ \ test\ 1\ after\ text
        fi
        printf "%s\n" \ \ \ \ \ \ \ \ after\ text\ indented
printf "%s\n" after\ text

rendered template

before text
        before text indented
        test 1 before text
        double true !
        test 1 after text
        after text indented
after text

The double true ! gets indentation-fixed TWICE, because it lives within two scripts blocks.

This is to encourage your to mix your scripts naturally within your text template, knowing that the indentation within script blocks will be removed in the rendered template ...

[TekWizely]

I mean, this is not really how I would write or read a bash script file

I do get your point, and maybe I was a bit overzealous with indenting the text printf statements to match the each line ...

I'll grind on this and see what I can do - I may add a command line option to better control that indentation - Not sure if it will make this PR or be a new feature ...

[flaix]

No worries. I would probably not do it in this PR, since it is a different topic. Also, it is just me so it really is not important.

As for the indentation I now better understand your intentions. I don't agree, because I have a different personal preference, probably coming from years of coding C/C++ and being used how preprocessor statements are handled there. So to me, personally, your intended way of a consolidated indentation is rather unusual and don't make things easier. I might still give it a try sometime and see if maybe I get used to it and see benefits.

Thanks anyhow for all your amazing work!

[TekWizely]

@flaix Thank you for your continued participation !

I'm going to merge this, but I'm still grinding on maybe adding a flag to disable the indentation fixing and just let all text statements print as they appear in the file.

keep an eye out for future PRs !