[Buildroot] [PATCH 2/5] download/git: re-run the backend with a lock to the git tree

[Yann E. MORIN]

On 2018-10-20 23:11 +0100, Arnout Vandecappelle spake thusly:
[--SNIP--]
>  So, my proposal would be:

As we discussed IRL, this is a good suggestion, but now that it has had
time to bounce a bit between my two still-working neurons, here are my
comments on it...

> 1. Remove the redundant temporary files from the download infra.

> 2. Replace the flock on the per-package download dir with a flock of a temporary
> file named ${filename}.lock. That way, we serialize just what needs to be
> serialized.

And who would be responsible to remove those lock files?

When the download runs OK, I can see it pretty easy, even though it is
not nice... But when the download gets interrupted (think Ctrl-C or
SIGTERM/SIGKILL from a CI...)?

I'm afraid we could end up with dozens stale such lock files cluttering
the (per-package) directory... Locking the per-package directory itself
was nice and dandy because it was expected to live on and stay.

I could see an alternative, which would be to lock the final file,
directly. After all, those lock are just advisory locks; they don't
actually prevent another process to write in the locked files at all.

However, that does not work, because then it would be an empty file, so
would not match its hashes, and the download wrapper would remove it,
thus freeing a competing build into creating that lock file again while
the first one is still trying to download it...

So we need to lock a file other than the destination file, and back to
square one...

> 3. Add a git-specific "global" lock, but do it directly from the dl-wrapper to
> keep the code simple.

And then, what should be locked?

If we lock a (say) 'git.lock' file in the per-package directory, but the
download is interrupted, the lock file would linger around. Since it is
only ever the only git-related lock file, that is not too bad. Even if
we had more per-backend locks, that is still just a few of them, and we
could make them hidden files (.git.lock).

If we rely on the fact that the backend would create a directory named
after itself (e.g. the git backend creates a directory named 'git'),
then the dl-wrapper could create that directory and handle it over to
the backend. But then, the backend should never ever remove that
directory (which we currently do in case the git tree is borked)), or a
competing download could grab the lock while the first would still be
attempting the download.

So, I am not fond of all those new lock files, which have the potential
to eventually clutter the download difrectory... :-(

(Note: I am not saying that the mere presence of lock files would
prevent future downloads; not at all, as we use flock(2) on them. I'm
just saying that stale lock files are ugly...)

> (in terms of patch order, obviously 2 and 3 have to be swapped).

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'