[Buildroot] [PATCH 03/14] pkg-infra: move the git download helper to a script

[Thomas De Schampheleire]

On Wed, Jul 2, 2014 at 7:22 PM, Thomas Petazzoni
<thomas.petazzoni at free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Wed, 2 Jul 2014 19:19:42 +0200, Thomas De Schampheleire wrote:
>
>> >> Thanks. I've verified that the (non-gzip'ed) tarball is identical to
>> >> what we had before, but I noticed that we no longer delete the temporary
>> >> repo in DL_DIR.
>> >>
>> >> Did you do that change on purpose? I don't think we want to keep it, do
>> >> we?
>> >
>> >Hum, I'm confused, didn't we say that we should no longer do any
>> >temporary thing in $(DL_DIR) in order to allow parallel builds of
>> >separate Buildroot instances to not mess up with each other? I think we
>> >said that the process should be:
>> >
>> > 1/ Clone the repo in $(BUILD_DIR)
>> > 2/ Create the tarball of the repo in $(BUILD_DIR)
>> > 3/ Move the tarball from $(BUILD_DIR) to $(DL_DIR) with a temporary
>> >    unique file name.
>> > 4/ Rename the tarball in $(DL_DIR) to its final name
>> >
>> >Steps (3) and (4) are separated so that if $(DL_DIR) and $(BUILD_DIR)
>> >are in separate filesystems, the rename to the final name remains an
>> >atomic operation.
>> >
>> >And yes, the git download helper from Yann doesn't seem to implement
>> >this logic (or I got lost with the variable names, which is very
>> >possible).
>>
>> This patch (3/14) only moves the existing logic to a separate script,
>> just like for the other helpers. Later patches in the same series
>> improve the logic. In particular, not using DL_DIR as scratchpad is
>> implemented in patch 11/14 http://patchwork.ozlabs.org/patch/365791/
>
> Ah, ok, thanks for the clarification. However that doesn't explain why
> the temporary repo in $(DL_DIR) is not being removed. That's a
> regression compared to the original code in the .mk file, no?

Patch 11 does:

-rm -rf "${repodir}"
+rm -rf "${repodir}" "${tmp_tar}" "${tmp_output}"
+exit ${ret}

and

-repodir="${BR2_DL_DIR}/${basename}"
+repodir="${basename}.tmp-git-checkout"
+tmp_tar="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
+tmp_output="$( mktemp "${output}.XXXXXX" )"

So from the code (haven't verified this now) there is no repo at all
in DL_DIR, so that repo does not need to be removed. The repo $repodir
_is_ removed, including all of the other temporary stuff tmp_tar and
tmp_output.

Peter, did you test this with all patches applied?

Thanks,
Thomas