Re: Your gnulib patch

Re: Your gnulib patch

[Jim Meyering]

On Tue, Dec 24, 2019 at 11:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> Hello, Jim
>
> We found your gnulib patch
> (http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=47cb657e)
> quite useful for us, as we encountered a same issue with a few million
> files under one directory.
>
> Is it possible for you to integrate the patch to glibc too? Our
> project doesn't use gnulib at all, so it would benefit more people if
> you could integrate it to glibc. :) I understand that would bring you
> more work, if I have your permission, I can help on that too. Please
> let me know how you would like to proceed.

Hi Cong Wang,

Thanks for your interest and the offer to contribute.

If you'd like that functionality in glibc, note that it would have to
use different function names, because the gnulib version of fts uses a
different API (e.g., numerous struct changes were required to remove
limitations in the glibc version).

You are welcome to pursue adding it to glibc, of course.

Re: Your gnulib patch

[Cong Wang]

On Wed, Jan 1, 2020 at 7:41 PM Jim Meyering <jim@meyering.net> wrote:
>
> On Tue, Dec 24, 2019 at 11:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > Hello, Jim
> >
> > We found your gnulib patch
> > (http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=47cb657e)
> > quite useful for us, as we encountered a same issue with a few million
> > files under one directory.
> >
> > Is it possible for you to integrate the patch to glibc too? Our
> > project doesn't use gnulib at all, so it would benefit more people if
> > you could integrate it to glibc. :) I understand that would bring you
> > more work, if I have your permission, I can help on that too. Please
> > let me know how you would like to proceed.
>
> Hi Cong Wang,
>
> Thanks for your interest and the offer to contribute.
>
> If you'd like that functionality in glibc, note that it would have to
> use different function names, because the gnulib version of fts uses a
> different API (e.g., numerous struct changes were required to remove
> limitations in the glibc version).

I understand I need to add at least ->fts_dirp to FTSENT in glibc,
but this seems fine as long as we only append to FTSENT? Users
should only dereference those documented fields of this struct.
Or am I still missing anything here?

(I get the point that gnulib is different as it is not a library so users
have to recompile for each update.)

>
> You are welcome to pursue adding it to glibc, of course.

Thanks for your kind response!

Re: Your gnulib patch

[Jim Meyering]

On Fri, Jan 3, 2020 at 2:43 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Wed, Jan 1, 2020 at 7:41 PM Jim Meyering <jim@meyering.net> wrote:
> >
> > On Tue, Dec 24, 2019 at 11:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > > Hello, Jim
> > >
> > > We found your gnulib patch
> > > (http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=47cb657e)
> > > quite useful for us, as we encountered a same issue with a few million
> > > files under one directory.
> > >
> > > Is it possible for you to integrate the patch to glibc too? Our
> > > project doesn't use gnulib at all, so it would benefit more people if
> > > you could integrate it to glibc. :) I understand that would bring you
> > > more work, if I have your permission, I can help on that too. Please
> > > let me know how you would like to proceed.
> >
> > Hi Cong Wang,
> >
> > Thanks for your interest and the offer to contribute.
> >
> > If you'd like that functionality in glibc, note that it would have to
> > use different function names, because the gnulib version of fts uses a
> > different API (e.g., numerous struct changes were required to remove
> > limitations in the glibc version).
>
> I understand I need to add at least ->fts_dirp to FTSENT in glibc,
> but this seems fine as long as we only append to FTSENT? Users
> should only dereference those documented fields of this struct.
> Or am I still missing anything here?

There are other required new members, e.g., to avoid O(N^2) effects,
where N is the depth of the tree, and a 2^16 limit on the depth of a
tree.

Adding a new API to glibc is a big project. I suggest that you
reconsider using gnulib.

Re: Your gnulib patch

[Cong Wang]

On Fri, Jan 3, 2020 at 3:31 PM Jim Meyering <jim@meyering.net> wrote:
>
> On Fri, Jan 3, 2020 at 2:43 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> >
> > On Wed, Jan 1, 2020 at 7:41 PM Jim Meyering <jim@meyering.net> wrote:
> > >
> > > On Tue, Dec 24, 2019 at 11:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > > > Hello, Jim
> > > >
> > > > We found your gnulib patch
> > > > (http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=47cb657e)
> > > > quite useful for us, as we encountered a same issue with a few million
> > > > files under one directory.
> > > >
> > > > Is it possible for you to integrate the patch to glibc too? Our
> > > > project doesn't use gnulib at all, so it would benefit more people if
> > > > you could integrate it to glibc. :) I understand that would bring you
> > > > more work, if I have your permission, I can help on that too. Please
> > > > let me know how you would like to proceed.
> > >
> > > Hi Cong Wang,
> > >
> > > Thanks for your interest and the offer to contribute.
> > >
> > > If you'd like that functionality in glibc, note that it would have to
> > > use different function names, because the gnulib version of fts uses a
> > > different API (e.g., numerous struct changes were required to remove
> > > limitations in the glibc version).
> >
> > I understand I need to add at least ->fts_dirp to FTSENT in glibc,
> > but this seems fine as long as we only append to FTSENT? Users
> > should only dereference those documented fields of this struct.
> > Or am I still missing anything here?
>
> There are other required new members, e.g., to avoid O(N^2) effects,
> where N is the depth of the tree, and a 2^16 limit on the depth of a
> tree.
>
> Adding a new API to glibc is a big project. I suggest that you
> reconsider using gnulib.

I already advised my colleagues to build with gnulib to address
this issue, before email you.

I am still trying to understand why adding new members to
FTSENT could break the API here. From my naive understanding,
appending to FTSENT is safe. Any hints?

Thanks.

Re: Your gnulib patch

[Jim Meyering]

On Tue, Jan 7, 2020 at 4:41 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Fri, Jan 3, 2020 at 3:31 PM Jim Meyering <jim@meyering.net> wrote:
> >
> > On Fri, Jan 3, 2020 at 2:43 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > >
> > > On Wed, Jan 1, 2020 at 7:41 PM Jim Meyering <jim@meyering.net> wrote:
> > > >
> > > > On Tue, Dec 24, 2019 at 11:13 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
> > > > > Hello, Jim
> > > > >
> > > > > We found your gnulib patch
> > > > > (http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=47cb657e)
> > > > > quite useful for us, as we encountered a same issue with a few million
> > > > > files under one directory.
> > > > >
> > > > > Is it possible for you to integrate the patch to glibc too? Our
> > > > > project doesn't use gnulib at all, so it would benefit more people if
> > > > > you could integrate it to glibc. :) I understand that would bring you
> > > > > more work, if I have your permission, I can help on that too. Please
> > > > > let me know how you would like to proceed.
> > > >
> > > > Hi Cong Wang,
> > > >
> > > > Thanks for your interest and the offer to contribute.
> > > >
> > > > If you'd like that functionality in glibc, note that it would have to
> > > > use different function names, because the gnulib version of fts uses a
> > > > different API (e.g., numerous struct changes were required to remove
> > > > limitations in the glibc version).
> > >
> > > I understand I need to add at least ->fts_dirp to FTSENT in glibc,
> > > but this seems fine as long as we only append to FTSENT? Users
> > > should only dereference those documented fields of this struct.
> > > Or am I still missing anything here?
> >
> > There are other required new members, e.g., to avoid O(N^2) effects,
> > where N is the depth of the tree, and a 2^16 limit on the depth of a
> > tree.
> >
> > Adding a new API to glibc is a big project. I suggest that you
> > reconsider using gnulib.
>
> I already advised my colleagues to build with gnulib to address
> this issue, before email you.
>
> I am still trying to understand why adding new members to
> FTSENT could break the API here. From my naive understanding,
> appending to FTSENT is safe. Any hints?

Changing FTSENT modifies the ABI, and that is a big deal and not
justified here, so you must provide a new set of functions (new API)
with functionality nearly identical to the fts* functions, but using
the new struct.