buildah: add 'cachi2.env' after any mount options to RUN

[owtaylor]

If you have a Dockerfile like:

RUN --mount=type=bind,from=somedir,to=/mnt/somewhere \
   do-something /mnt/somewhere

then that will get mangled for a prefetch enabled build to:

RUN . /cachi2/cachi2.env && --mount=type=bind,from=somedir,to=/mnt/somewhere \
   do-something /mnt/somewhere

Which is of course invalid. This patch uses a more complicated sed invocation to add the cache2.env after any leading options, and handling line continuations.

[chmeliik]

Could you explain a bit more about how this works? I ran it through ChatGPT, but still don't fully understand how and why it works.

[owtaylor]

Could you explain a bit more about how this works? I ran it through ChatGPT, but still don't fully understand how and why it works.

I pushed a slightly updated version with minor improvements, let me explain that:

          sed -E -i \

Inplace, using -E to avoid \( and \)

+              -e 'H;1h;$!d;x' \

Idiom to read the entire file into the pattern space (https://unix.stackexchange.com/questions/533277/how-do-i-process-the-whole-file-in-one-buffer-in-sed-without-gnu-z-option) - I was using a different idiom in the last version that is slightly buggy, though in a way that doesn't matter here.

+              -e 's@^\s*(run((\s|\\\n)+-\S+)*(\s|\\\n)+)@\1. /cachi2/cachi2.env \&\& \\\n    @igM'

Look for lines that start with:

• optional whitespace
• "run" (case insensitive)
• Zero or more of:
  • mandatory whitespace (including escaped \n)
  • a dash followed by non-space characters
• mandatory whitespace (including escaped \n)

And add the \1. /cachi2/cachi2.env \&\& \\\n after that.

Flags:

• i - case insensitive
• g - replace all matches (since we are matching against the entire file)
• M - Match ^ after any newline in addition to at the beginning of the pattern space

So, if you say have:

RUN \
        --mount=type=bind,from=somedir,to=/mnt/somewhere \
   do-something /mnt/somewhere

It will match all the way up to (but not including) the d of do-something, and then insert

RUN \
        --mount=type=bind,from=somedir,to=/mnt/somewhere \
   . /cachi2/cachi2.env && \
   do-something /mnt/somewhere

Do you want the stackoverflow link in a comment? Any other details?

[chmeliik]

Perfect, thanks 👍
Yeah, the stackoverflow link could be helpful.

I was more or less able to guess what was happening, but having this detailed explanation here for posterity is great. And I did miss some details before, e.g. the \\\n being a backslash at the end of a line (my eyes just parsed it as a newline)

[chmeliik]

/ok-to-test

[chmeliik]

LGTM 🎉

(the stackoverflow link could optionally be helpful)

[owtaylor]

New version with the link added

[chmeliik]

/retest

[chmeliik]

Looked into the test failures but they didn't make any sense. Trying again after rebasing on main

[chmeliik]

Finally found the error message in the e2e-test output this time:

[e2e-tests : e2e-test]   sed: no input files

Suggested change

	-e 's@^\s*(run((\s|\\\n)+-\S+)*(\s|\\\n)+)@\1. /cachi2/cachi2.env \&\& \\\n @igM'
	"$dockerfile_path"
	-e 's@^\s*(run((\s|\\\n)+-\S+)*(\s|\\\n)+)@\1. /cachi2/cachi2.env \&\& \\\n @igM' \
	"$dockerfile_path"

[owtaylor]

Bah - obviously I never tested the tweaked version in real life, just that fragment in isolation :-( - thank goodness for the e2e tests! (I don't have log access, so I couldn't see how they were failing.) I've now pushed a fixed up version, also adding the changes for 0.2/ as well.

[chmeliik]

#1160 is about to merge (if it passes tests in the merge queue 🤞 ). It adds versions 0.2 of every buildah task. Please move these changes to version 0.2 afterwards, otherwise only version 0.1 will have them