[bitbake-devel] [PATCH 0/9] Always use the url specified in the recipe as a base for the git shallow tarball naming

[Urs Fässler]

Hi Richard,
I have improved my patches but like to have your Input before I commit
them. Please see my questions/reasoning below.

Thanks
Urs

On Wed, 2018-07-25 at 10:57 +0200, Urs Fässler wrote:
> Thank you for your feedback.
> 
> On Mon, 2018-07-23 at 23:00 +0100, Richard Purdie wrote:
> > On Mon, 2018-07-23 at 17:42 +0200, Urs Fässler wrote:
> > > The existing behavior was to use the url where the repository was
> > > cloned from.
> > > It happened that the tarball was not found when a mirror rewrite
> > > rule
> > > was active.
> > > 
> > > We now use the url specified in the recipe to name the shallow
> > > tarball. It fixes that the tarball is not found under certain
> > > conditions. In addition, the naming is independent of
> > > network/server
> > > failure and the mapping of the name is easier to understand.
> > 
> > I'm not sure why but throughout this patch series you've used
> > function
> > names and variables prefixed with "__". We don't do this with any
> > of
> > our other code so it seems out of keeping with the rest of the
> > coding
> > style.
> 
> I will fix that.
> There are methods in the git fetcher (same file) that are prefixed
> with
> one underscore. Is this the way to go or do you prefer no prefixing
> underscores at all?
> 
> > With regard to the function "__has_up_to_date_clonedir" you added,
> > its
> > very unclear why some checks would go in that function and some
> > checks
> > would go in the other. The commit description helps understand it a
> > bit
> > more but that doesn't help the function naming or someone looking
> > at
> > the code in future.
> 
> I will fix that too.
> The difficulty with this one is, that the use of "need_update" is
> most
> certainly a misuse in this context. I can improve the patches to make
> the misuse more clear, but it changes only a bit on the end result.
> 
> I am planning to resend only the patches that handle the cases when
> the
> sources to unpack are not found (up to Patch 5/9 in this series)
> first.
> The patches to solve the root of problem will follow separately.
> 
> > Finally, I'm still not convinced that passing around the original
> > url
> > and forcing the original url naming into any mirroring code is the
> > right solution to the problem. I said that at the start and looking
> > at
> > this code change, I'm still not convinced this is right. Part of
> > the
> > reason I continue to believe that is you just added a N*N testing
> > problem to the fetcher where the fetchers now behave differently
> > depending on two urls passed in rather than just one :(.
> 
> Maybe it helps when I describe our use case and problem.
> 
> Our Yocto build machines have no access over the git protocol to the
> servers. We use mirror rewrite rules to use git over https to access
> the servers. Some server provide the git over http repository under a
> different URL as when accessed over the git protocol. Since the URL
> where we cloned the repository from is used to name the tarball, we
> end
> up with different names of the tarballs depending of the availability
> of some servers.
> 
> To be able to build our Image years later we archive the generated
> tarballs. One way to store them is to use Amazon S3 which does not
> natively support symlinks. To be able to access the tarballs from S3
> over http, we use a mirror rewrite rule.
> 
> At the moment we have different scenarios where it fails, all of them
> a
> bit difficult to reproduce. The common root of the problems is, that
> the tarballs are not found because they are searched with the wrong
> name.
> 
> I support your point that the same things should be solved with the
> same mechanism. But in our case, the tarballs are not the same thing
> as
> the local git clones. From our view as Bitbake user, the local git
> clones are Bitbake internals whereas the tarballs are artifacts we
> get
> out of Bitbake and use with third-party systems.
> 
> As a Bitbake user, I expect that the name of the tarball only depends
> on the URL I specify in the recipe. I certainly do not expect that
> the
> name is different depending on the availability of some servers since
> this is a transport layer issue.
> 
> I think both solutions (symlinking tarballs and using recipe URL for
> naming) solve the problem equally. We prefer the recipe URL solution
> for better compatibility with third party systems.
> 
> But then I see that the symlink solution is the one that nicely fits
> into Bitbake. I have to check if this is a feasible way to go for
> us. 
> 
> Cheers
> Urs
>