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

["Urs Fässler" <urs.fassler@bbv.ch>]

ping

On Fri, 2018-08-03 at 08:48 +0200, Urs Fässler wrote:
> 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
> >