cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [bagder/curl] 13606b: build: make use of 93 lib/*.c renamed files

From: Yang Tse <yangsita_at_gmail.com>
Date: Thu, 3 Jan 2013 18:04:53 +0100

Replying to all in last msg in thread I've read in order to keep a
linear thread, even at the expense of making this msg longer...

----------------------------------------
@Daniel...
----------------------------------------

On Thu, Jan 3, 2013, Daniel Stenberg wrote:
> On Thu, 3 Jan 2013, Yang Tse wrote:
>
>> 2) Specifically, renaming setup.h and setup_once.h to curl_setup.h and
>> curl_setup_once.h was needed to break bidirectional interdependency on
>> c-ares files when building with c-ares in-tree. This change alone modified
>> same big number of files.
>
>
> Building c-ares in-tree is an ancient tradition that we should NOT preserve
> at the expense of anything in the curl project. Not even I build c-ares
> in-tree anymore and we shouldn't encourage anyone in doing so either. I
> would much prefer we just proclaimed it not supported anymore and have a
> nicer build environment for curl.

Specific renaming of setup.h and setup_once.h does not actually force
the renaming of the other files. It only implies a libcurl-wide
modification of the files that already did an "#include "setup.h" or
"#include "setup_once.h". I believe this specific change is perfectly
acceptable and desirable, even if the massive renaming is finally
reverted.

>> 3) Points above are in preparation of at-some-future point splitting
>> of curl_setup_once.h into two headers, one that is required to be
>> included in libcurl's sources before other libcurl headers and another
>> one that has to be included nearly last although not for the same
>> reasons an curl_memdebug.h
>
>
> The splitting of a single header or two surely can't motivate the renaming
> of 190 *other* files?

True, the splitting doesn't actually motivate the renaming. It would
only justify the modification of the 190 *other* files.

>> 4) When human reading libcurl's code it is much easier to know if a
>> header is a libcurl header or not.
>
>
> Sure, I'll give you that. But on the other hand, you've now alienated all of
> us old-timers from our own code base. What we formerly knew so intimately is
> now gone. I find this to be a *really* hard blow against my productivity. I
> know (and use) a large amount of header names and off the top of my head and
> I use them frequently.

If the renaming had been done in some arbitrary way I would fully
understand that previous intimate knowledge would be gone. But the
renaming that has been done is simply prepending 'curl_' to *.c and
*.h source files in lib subdirectory that did not have it.

I accept that it may be a bit disturbing for us to type the new names
instead of the old ones until we would get used to them. But it is
only a matter of remembering that _all_ of them are 'curl_' prepended.

Although I understand that initially it might be quite distracting,
saying that previous knowledge is gone is probably a bit of an
exaggeration.

>> 2) When debugging libcurl or apps that use it with debuggers that are
>> capable of showing the name of the source file it is much easier to know if
>> one is stepping in libcurl's code or elsewhere.
>
>
> Yes sure, but that hasn't ever been a problem either and you've instead
> introduced problems that the file names have turned very long and instead
> make outputs wrap and be harder to read.

Yes file names in lib subdirectory which didn't start with 'curl_'
would now be 5 characters longer. I don't know which output is
wrapping for you, which may perfectly happen, but for example here on
an 80 chars wide terminal screen when executing 'ls -l lib/*' the only
file name that wraps is curl_http_negotiate_sspi.c.

Is that the kind of wrapping that you are mentioning here or something
else I'm overlooking or unaware?

>> 3) On core dumps, and fatal happenings which trace the source file
>> name of where the problem has been triggered, it becomes much easier
>> for not expert souls to figure out if it is a libcurl problem or not.
>
>
> I would be surprised if this will help anyone. If you're clever enough to
> read a back trace of a core dump to get info from it, I'm sure you can
> figure out which library the functions belong to.

Final users of apps which use libraries built debug enabled don't need
to have a clue about programming and much less about debugging, but if
the app fails in a way the OS is capable of telling the source file
where the thing has 'failed' it will help the app developer to figure
out which 'component' has failed. For example, knowing that the
problem is not in a 'curl_*' file would automatically discard libcurl
as the culprit.

Not perfect, but I believe it can be helpful.

>> 4) Also makes naming of *.c files equal to respective *.h one.
>
>
> Right, but I am against both renames so that's not a benefit.

OK, I count you as against prepending 'curl_' to lib/*.[ch] which
don't already have it.

> Further downsides with the renames:
>
> * Patches for older curl versions now suddenly don't apply anymore and will
> give us (and others) much more work to apply. This will not only affect a
> couple of patches that I have pending in the mailing list but also in the
> bug tracker. There's also about a bazillion distros out there with distro
> specific patches that suddenly get version-specific file name changes
> (where the patches could otherwise apply to multiple versions).

Adapting the patch should be as easy as modifying the lib/*.[ch] diffs name.

> * History tracking/git log is much harder and annoying with file renames. So
> while it can be handled, things like the github's history breaks and
> bisecting things will require more work.

While true that git history tracking is a bit inconvinient on renames.
The commits of these renames have been done in a way that history
tracking should actually represent no problem. These commits which do
the renames only rename the files, zero changes in such commits except
for the file renaming. So why doesn't git pull the history of the
original file into the renamed one? After all all the info is there
for it to use.

Additionally we have the multi-allways patch that is going into
libcurl soonish. Trying to bisect across that patch will make little
sense at all. libcurl internals will be definitively changed that if
it has not already happened with the 'bundles connection cache'
modification. Unless I misinterpreted you I thought libcurl 7.29.0 was
going to be a no-return point. I may be wrong but i believe that it
might carry some external functional behavior change in 'pingpong'
based protocols (ftp, pop3, imap, smtp).

So, if history was to be 'broken' at some point, now might be the best
moment to do so.

> Sorry, but my vote goes to reverting the renaming of the headers and the
> source files.

I count it as such. Let us give ourselves 24, 48 hours to let other
express themselves, if they desire so.

----------------------------------------
@Tor...
----------------------------------------

On Thu, Jan 3, 2013, Tor Arntsen wrote:
>
> I already commented on github on the patch itself,

Could you provide a link to this, or is it some github 'internal' list?

> but I want to add
> to what Daniel said above - we use Git at work, and although Git
> itself tries to support history tracking with renames, it can never
> work truly well. What happens in practice is that you suddenly have a
> short history (for 'git log' and tons of other commands), and then
> you'll have to try combinations of -M and -C and hope that you can get
> to the historic info you need. The one thing I have learned to regret
> and avoid is massive renames and moves, it only creates grief. And
> yes, forget about applying existing patches in a general way.

Unless you say the contrary I'll count you as opposed to the renaming.

If we were to revert the renaming commits, which would be the best
approach in order to keep github's master repo history unchanged?

In other words, is it possible, and how, to make all commits following
commit ec691ca34b disappear from github repo, even if we have to
manually re-commit other 23 unrelated commits that have been
intermixed sinche the renaming of the header files?

Or is history breakage already done no matter what we do? :-((

----------------------------------------
@Kamil...
----------------------------------------

On Thu, Jan 3, 2013, Kamil Dudka wrote:
>
> We often backport upstream fixes for
> ancient versions of (lib)curl, which are still supported in the Enterprise
> Linux. I know this is our business, but the less time we spent on backporting
> fixes the more time we have to help upstream.

Please clarify if you oppose the renaming or not.

----------------------------------------
@Gisle...
----------------------------------------

On Thu, Jan 3, 2013, Gisle Vanem wrote:
> "Yang Tse" <yangsita_at_gmail.com> wrote:
>
>> 2) When debugging libcurl or apps that use it with debuggers that are
>> capable of showing the name of the source file it is much easier to
>> know if one is stepping in libcurl's code or elsewhere.
>
>
> I think that's a good argument for renaming.

Unless you say the contrary I'll count you as agreeing to the renaming.

>> From your message http://curl.haxx.se/mail/lib-2013-01/0012.html I'm
>> unable to fully understand if Metaware+Pharlap-DOS-extender was
>> capable of working before this commit, or if support for it was
>> already broken.
>
>
> High-C + Pharlap-DOSX + Watt-32 tcp/ip used to work fine (approx. 5 years
> since I checked). But this combo was broken 3 years ago. Is there's some
> interest in making it work again? I'm looking into it.

Ok, it was broken long time ago.

On the other hand commit dfe4769157 with test case 1221 might have
gone unnoticed, but it is the foundation or initial work to allow in a
not too far future building libcurl and curl even on 8+3 OS's while
still allowing arbitrary names for libcurl sources. Test case 1222
will follow soonish. :-)

-- 
-=[Yang]=-
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html
Received on 2013-01-03