[WIP] tool_parsecfg: Support .netrc and .curlrc on Windows

[captain-caveman2k]

Support and prefer .curlrc over _curlrc in Windows. Other OSes are unaffected by this change; so _curlrc on DOS and .curlrc on all others.

I know .curlrc on Windows has been discussed before and without checking the mailing lists I can't remember the last time it was. However this is my take on trying to support .curlrc on Windows as well as maintaining support for _curlrc (as Windows doesn't have the same limitation as DOS).

Note: I haven't worried about supporting older versions of Windows that don't support long file names - ie versions prior to Windows 95 that are not Windows NT based.

In summary it performs the check for the CURLRC file in both the 'home' folder and the current executable directory as per the current behaviour, but loops round this code twice. The first time looking for .curlrc and then if that fails looking for _curlrc the second time.

It fixes a long known "git for windows" bug bear (as it's been in their release notes since I have been involved with curl - and using git).

If the general consensus is that we should proceed with 'fixing' this then I would propose something similar for _netrc too.

I would appreciate feedback and comments.

Many thanks

[gvanem]

BTW.
Having to keep both a _netrc and .netrc in sync, is too much hassle IMHO. So could we use .netrc on Windows too?

[jay]

I don't think it's necessary. It has long been documented to use _curlrc and _netrc and I don't think there are any problems from that so why change it? But I'm curious about the git for windows bug, can you give a reference where it talks about _curlrc?

[bagder]

Due to the mention of git for windows, maybe @dscho has a thought?

[dscho]

I like the idea, and suggested a couple of changes that would IMO improve the PR.

And I agree with the concern that it does not make much sense to do this only for _curlrc but not for _netrc. The more important "known issue" of Git for Windows is about .netrc being ignored, in favor of _netrc.

[dscho]

Makes sense to me!

[dscho]

I have to say that this indentation change makes the patch quite hard to review, at least for me.

Besides, I do not like that CURLRC_LEGACY is defined but then used in anything but an #ifdef CURLRC_LEGACY code block.

Therefore, I would like to suggest a different strategy, based on a simple and elegant goto. In particular:

• add a label, say, retry_reading_curlrc: instead of the for statement,
• Replace the comment /* Don't bother checking if it exists - we do that later */ and the line following it by something like this:

file = fopen(filebuffer, FOPEN_READTEXT);
if(file != NULL) {
  filename = filebuffer;
}
#ifdef CURLRC_LEGACY
else if (strcmp(CURLRC_LEGACY, filename)) {
    filename = CURLRC_LEGACY;
    goto retry_reading_curlrc;
}
#endif

That will be much easier to review, and therefore it will be much harder for bugs to creep in.

[dscho]

Due to the mention of git for windows, maybe @dscho has a thought?

Thanks for the ping! I would love to be able to get rid of that "Known Issues" entry. And I agree that it makes a total lot of sense to look for .curlrc first, falling back to looking for _curlrc. And likewise .netrc, with a fall-back to _netrc (as I mentioned in my review, I deem .netrc even more important than .curlrc).

[dscho]

It has long been documented to use _curlrc and _netrc and I don't think there are any problems from that so why change it?

@jay just because users acquiesce with the status quo does not mean that we should refrain from improving it...

But I'm curious about the git for windows bug, can you give a reference where it talks about _curlrc?

It actually talks about _netrc: https://github.com/git-for-windows/build-extra/blob/master/ReleaseNotes.md#known-issues

[captain-caveman2k]

Due to the mention of git for windows, maybe @dscho has a thought?

Thanks for the ping! I would love to be able to get rid of that "Known Issues" entry. And I
agree that it makes a total lot of sense to look for .curlrc first, falling back to looking for
_curlrc. And likewise .netrc, with a fall-back to _netrc (as I mentioned in my review, I deem
.netrc even more important than .curlrc).

No problem - that was next on my list but I must admit I forgot to mention it in the PR description :(

[bagder]

I'll just voice my support for making sure that curlrc and netrc get the same treatmeant with regards to leading dot/underscore to reduce the amount of possible confusion.

[captain-caveman2k]

I'll just voice my support for making sure that curlrc and netrc get the same treatmeant with regards to leading dot/underscore to reduce the amount of possible confusion.

No problem - I was going to work on that as a separate patch set but I think with hindsight I will update the title of this PR and make it part of this patch set.

[bagder]

This was replaced by #4230