[PATCH 0/1] fetch2: remove "." in the end

[PATCH 0/1] fetch2: remove "." in the end

[Robert Yang]

The following changes since commit c6d50b272936b61b98a056ea2e1de5d0f4ef53ce:

  bitbake: lib/bb/build.py: remove task flag in deltask() (2016-06-23 14:26:33 +0100)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib rbt/dot
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=rbt/dot

Jason Wessel (1):
  fetch2: remove "." in the end

 bitbake/lib/bb/fetch2/__init__.py | 2 ++
 bitbake/lib/bb/fetch2/git.py      | 2 ++
 2 files changed, 4 insertions(+)

-- 
2.8.2

[PATCH 1/1] fetch2: remove "." in the end

[Robert Yang]

From: Jason Wessel <jason.wessel@windriver.com>

The filename can't be "foo." for MS Windows filesystem, it will renamed
to "foo" automatically, so we can't upload sources like "foo." to the
Windows server, remove "." in the end will fix the problem.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
 bitbake/lib/bb/fetch2/__init__.py | 2 ++
 bitbake/lib/bb/fetch2/git.py      | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
index b6fcaaa..1633c81 100644
--- a/bitbake/lib/bb/fetch2/__init__.py
+++ b/bitbake/lib/bb/fetch2/__init__.py
@@ -865,6 +865,8 @@ def build_mirroruris(origud, mirrors, ld):
     replacements["PATH"] = origud.path
     replacements["BASENAME"] = origud.path.split("/")[-1]
     replacements["MIRRORNAME"] = origud.host.replace(':','.') + origud.path.replace('/', '.').replace('*', '.')
+    if replacements["MIRRORNAME"].endswith('.'):
+        replacements["MIRRORNAME"] = replacements["MIRRORNAME"][:-1]
 
     def adduri(ud, uris, uds, mirrors):
         for line in mirrors:
diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 59827e3..e669efc 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -142,6 +142,8 @@ class Git(FetchMethod):
         gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/', '.').replace('*', '.'))
         if gitsrcname.startswith('.'):
             gitsrcname = gitsrcname[1:]
+        if gitsrcname.endswith('.'):
+            gitsrcname = gitsrcname[:-1]
 
         # for rebaseable git repo, it is necessary to keep mirror tar ball
         # per revision, so that even the revision disappears from the
-- 
2.8.2

Re: [PATCH 1/1] fetch2: remove "." in the end

[Richard Purdie]

On Fri, 2016-06-24 at 00:55 -0700, Robert Yang wrote:
> From: Jason Wessel <jason.wessel@windriver.com>
> 
> The filename can't be "foo." for MS Windows filesystem, it will
> renamed
> to "foo" automatically, so we can't upload sources like "foo." to the
> Windows server, remove "." in the end will fix the problem.

This patch on its own is probably ok. What I worry about is that if I
merge this, I'll then get all the follow ups which for example force
lower or upper case everywhere, remove ":" characters from all
filenames (including sstate?), remove various other characters and so
on.

We don't run on windows filesystems and we're not likely ever to be
able to.

So what are we aiming for here?

Cheers,

Richard

Re: [PATCH 1/1] fetch2: remove "." in the end

[Robert Yang]

On 06/24/2016 09:15 PM, Richard Purdie wrote:
> On Fri, 2016-06-24 at 00:55 -0700, Robert Yang wrote:
>> From: Jason Wessel <jason.wessel@windriver.com>
>>
>> The filename can't be "foo." for MS Windows filesystem, it will
>> renamed
>> to "foo" automatically, so we can't upload sources like "foo." to the
>> Windows server, remove "." in the end will fix the problem.
>
> This patch on its own is probably ok. What I worry about is that if I
> merge this, I'll then get all the follow ups which for example force
> lower or upper case everywhere, remove ":" characters from all
> filenames (including sstate?), remove various other characters and so
> on.
>
> We don't run on windows filesystems and we're not likely ever to be
> able to.
>
> So what are we aiming for here?

We don't run bitbake on windows, either. Our problem is that we put
the downloaded sources on a windows server, and the filename mismatches
when the name is "foo.", so it would fail to download.

// Robert

>
> Cheers,
>
> Richard
>

Re: [PATCH 1/1] fetch2: remove "." in the end

[Jason Wessel]

On 06/24/2016 08:15 AM, Richard Purdie wrote:
> On Fri, 2016-06-24 at 00:55 -0700, Robert Yang wrote:
>> From: Jason Wessel <jason.wessel@windriver.com>
>>
>> The filename can't be "foo." for MS Windows filesystem, it will
>> renamed
>> to "foo" automatically, so we can't upload sources like "foo." to the
>> Windows server, remove "." in the end will fix the problem.
> This patch on its own is probably ok. What I worry about is that if I
> merge this, I'll then get all the follow ups which for example force
> lower or upper case everywhere, remove ":" characters from all
> filenames (including sstate?), remove various other characters and so
> on.
>
> We don't run on windows filesystems and we're not likely ever to be
> able to.
>
> So what are we aiming for here?


It is compatibility with browsing and copying the bitbake/oe directory structures + the download cache.  We certainly don't expect to be building directly on Windows with a native bitbake. Today however, you can directly use git on Windows and building with the cross API's from a generated SDK.

Cheers,
Jason.

Re: [PATCH 1/1] fetch2: remove "." in the end

[Richard Purdie]

On Mon, 2016-06-27 at 08:46 -0500, Jason Wessel wrote:
> On 06/24/2016 08:15 AM, Richard Purdie wrote:
> > On Fri, 2016-06-24 at 00:55 -0700, Robert Yang wrote:
> > > From: Jason Wessel <jason.wessel@windriver.com>
> > > 
> > > The filename can't be "foo." for MS Windows filesystem, it will
> > > renamed
> > > to "foo" automatically, so we can't upload sources like "foo." to
> > > the
> > > Windows server, remove "." in the end will fix the problem.
> > This patch on its own is probably ok. What I worry about is that if
> > I
> > merge this, I'll then get all the follow ups which for example
> > force
> > lower or upper case everywhere, remove ":" characters from all
> > filenames (including sstate?), remove various other characters and
> > so
> > on.
> > 
> > We don't run on windows filesystems and we're not likely ever to be
> > able to.
> > 
> > So what are we aiming for here?
> 
> 
> It is compatibility with browsing and copying the bitbake/oe
> directory structures + the download cache.  We certainly don't expect
> to be building directly on Windows with a native bitbake. Today
> however, you can directly use git on Windows and building with the
> cross API's from a generated SDK.

You haven't really answered my question though. If we fix this, how
many other issues are we going to run into? Are we for example going to
need to change the sstate filename field separator? Are there other
filename issues we'll run into. Typically, someone sends a simple patch
like this, then a couple more and then we suddenly find we've committed
to rewriting half the system to "support windows filesystems".

I'm going to refuse to do this piecemeal. I'd like a thought out
proposal about exactly which files we need to support on windows and
how much of bitbake we're expecting to be able to use (or which class
code/tools) before we start adding patches.

Cheers,

Richard

Re: [PATCH 1/1] fetch2: remove "." in the end

[Jason Wessel]

On 06/27/2016 08:51 AM, Richard Purdie wrote:
> On Mon, 2016-06-27 at 08:46 -0500, Jason Wessel wrote:
>> On 06/24/2016 08:15 AM, Richard Purdie wrote:
>>> On Fri, 2016-06-24 at 00:55 -0700, Robert Yang wrote:
>>>> From: Jason Wessel <jason.wessel@windriver.com>
>>>>
>>>> The filename can't be "foo." for MS Windows filesystem, it will
>>>> renamed
>>>> to "foo" automatically, so we can't upload sources like "foo." to
>>>> the
>>>> Windows server, remove "." in the end will fix the problem.
>>> This patch on its own is probably ok. What I worry about is that if
>>> I
>>> merge this, I'll then get all the follow ups which for example
>>> force
>>> lower or upper case everywhere, remove ":" characters from all
>>> filenames (including sstate?), remove various other characters and
>>> so
>>> on.
>>>
>>> We don't run on windows filesystems and we're not likely ever to be
>>> able to.
>>>
>>> So what are we aiming for here?
>>
>> It is compatibility with browsing and copying the bitbake/oe
>> directory structures + the download cache.  We certainly don't expect
>> to be building directly on Windows with a native bitbake. Today
>> however, you can directly use git on Windows and building with the
>> cross API's from a generated SDK.
> You haven't really answered my question though. If we fix this, how
> many other issues are we going to run into? Are we for example going to
> need to change the sstate filename field separator? Are there other
> filename issues we'll run into. Typically, someone sends a simple patch
> like this, then a couple more and then we suddenly find we've committed
> to rewriting half the system to "support windows filesystems".


This is a problem that was found in steady state maintenance from 5 years worth of using Windows as a cross application building environment from a derived platform build from a Linux environment.   There is no foreseeable plan to change what is there.   This is a "bug fix" to a problem which has existed for some time now. Unfortunately Robert blindly sent a patch on my behalf from an internal pastebin and it did not include how the problem was found or any of the original suggested solution discussion.   In that case the patch should have come from him and not me.

That being said I'll bring up some of the points of how the problem was discovered.

1) A git URI ended in a slash in a single recipe.

     My personal contention is that this is just wrong.
        A) Git didn't always have support for the "/" at the end
        B) In some cases this can not be used a push reference

     Evidence in the community shows that since 2014:
        A) git internally drops the slash and the right thing
        B) git hub started allowing https push with the trailing slash

2) Originally I proposed just adding a warning or error to bitbake for
      a trailing "/" and then fixing the single problematic recipe
      out of the 2900 we parse.

3) The OE download cache uses a direct translation of the URI
     A) Because there is community support for the trailing / on URI
          perhaps fixing the cache is a better approach
     B) The fetcher cache creates a bare clone of a git which
          in turn does not need the trailing slash
     C) I created and tested a simple patch with the fetcher

4) Robert sent this patch from one of my pastebin possibilities instead of
     having an open discussion on the topic first.


>
> I'm going to refuse to do this piecemeal. I'd like a thought out
> proposal about exactly which files we need to support on windows and
> how much of bitbake we're expecting to be able to use (or which class
> code/tools) before we start adding patches.

I do not believe there is any proposal to write.  In general what we have for Windows today has worked fine.  It has been working for the last 5 years.   We regression test any changes to the master branch against the same Windows SDK policy we have been using and are not making any changes to that policy or adding additional functionality.

Do you believe we are standing on some kind of slippery slope?

Prior to hitting this problem with the single recipe we did not even have a test for a directory ending in a dot.  Case folding on the other hand has been tested and the limitations and work arounds are documented and already implemented.  I see no need to change anything else.  If you look at this another way, a couple line fix to a routine where a corner case was found after 5 years of use really isn't too bad.

Cheers,
Jason.

Re: [PATCH 1/1] fetch2: remove "." in the end

[Mark Hatle]

Based on some feedback on IRC.  I tried to move this into the git.py
'urldata_init' function.

This was pretty simple and appeared to work on the surface.  I then added a test
case to the fetch.py test cases.  At which point I can not reconcile the work.

In the test harness, around line 296 -- 'def test_uri', the system compares the
input URI, to the generated URI, i.e.:

        "git://example.net/path/example/": {
            'uri': 'git://example.net/path/example',
            'scheme': 'git',
            ...

The URI function and the fetch processing run two different paths.  Since the
URI class has no concept of special processing, the 'uri' it ends up with is
always "git://example.net/path/example/".  However, the fetchers processing,
after stripping the trailing '/' from the ud.path and then reconstructing the
URI ends up with "git://example.net/path/example".  And a test case failure I
can't get around.

So I have a couple of questions:

1) Why do we have both a URI and encodeurl/decodeurl set of functions?  It seems
like we have multiple ways of getting exactly the same data.

2) Is there a flaw in this test harness, that it verifies that element 1 and
element 2:uri always have to match?

The commit that highlights this issue is available as:

http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/commit/?h=mgh/bitbake-git&id=5e45f3dea413f5b95fd8a872d48b72c11df15680

--Mark

On 6/24/16 2:55 AM, Robert Yang wrote:
> From: Jason Wessel <jason.wessel@windriver.com>
> 
> The filename can't be "foo." for MS Windows filesystem, it will renamed
> to "foo" automatically, so we can't upload sources like "foo." to the
> Windows server, remove "." in the end will fix the problem.
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  bitbake/lib/bb/fetch2/__init__.py | 2 ++
>  bitbake/lib/bb/fetch2/git.py      | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/bitbake/lib/bb/fetch2/__init__.py b/bitbake/lib/bb/fetch2/__init__.py
> index b6fcaaa..1633c81 100644
> --- a/bitbake/lib/bb/fetch2/__init__.py
> +++ b/bitbake/lib/bb/fetch2/__init__.py
> @@ -865,6 +865,8 @@ def build_mirroruris(origud, mirrors, ld):
>      replacements["PATH"] = origud.path
>      replacements["BASENAME"] = origud.path.split("/")[-1]
>      replacements["MIRRORNAME"] = origud.host.replace(':','.') + origud.path.replace('/', '.').replace('*', '.')
> +    if replacements["MIRRORNAME"].endswith('.'):
> +        replacements["MIRRORNAME"] = replacements["MIRRORNAME"][:-1]
>  
>      def adduri(ud, uris, uds, mirrors):
>          for line in mirrors:
> diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
> index 59827e3..e669efc 100644
> --- a/bitbake/lib/bb/fetch2/git.py
> +++ b/bitbake/lib/bb/fetch2/git.py
> @@ -142,6 +142,8 @@ class Git(FetchMethod):
>          gitsrcname = '%s%s' % (ud.host.replace(':', '.'), ud.path.replace('/', '.').replace('*', '.'))
>          if gitsrcname.startswith('.'):
>              gitsrcname = gitsrcname[1:]
> +        if gitsrcname.endswith('.'):
> +            gitsrcname = gitsrcname[:-1]
>  
>          # for rebaseable git repo, it is necessary to keep mirror tar ball
>          # per revision, so that even the revision disappears from the
>