Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

embryo for tracing options from config files #13295

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

bagder
Copy link
Member

@bagder bagder commented Apr 5, 2024

To help users understand when command line options are set by a "config file". In the default .curlrc file or in files specified with -K, this change outputs a trace on stderr. To enable the trace, you either use -v / --verbose as the first command line argument or you set the CURL_CONFIG_DEBUG environment variable.

This change also allows the user to set an environment variable to change the name (and path) of the default .curlrc file curl uses.

  • decide if these are good ideas
  • decide on naming
  • documentation
  • tests

why -v as first argument?

The reason is that our command line parser parses and acts on the content in the same phase, so we cannot easily check the entire command line for a verbose option without also reading all the specified config files etc.

This is basically the best idea I came up with that avoids a new option and still works.

Environment variables

CURL_CONFIG_DEBUG set to anything enables config file tracing

CURL_RC set to a (full) file name to read by default instead of $HOME/.curlrc

@tlhackque
Copy link

As I noted on the curl=users list, a simpler option is to simply look at the command name in argv[0]. (Do check that argc >= 1)

A user can simply soft-link curl to curl-v (or whatever - this seems appropriate). If the name is *-v, enable -v early (which then traces .curlrc).

No special parsing or command line position restriction needed. No mystery environment variable.

There are very few platforms left that don't support softlinks - and in that case, a user can copy curl{.exe} to curl-v{.exe} for the same effect.

Optionally, installation can do the softlink (or copy).

I think this is both more straightforward to implement and less work for the user.

Precedents abound - e.g. view for vi readonly.

@bagder
Copy link
Member Author

bagder commented Apr 5, 2024

curl-v

... it could then be tempting to allow arbitrary command line options like that. If the name contains a dash, parse everything from there as command line arguments... curl-O and curl-s comes to mind. And combos: curl-sv ...

@tlhackque
Copy link

curl-v

... it could then be tempting to allow arbitrary command line options like that. If the name contains a dash, parse everything from there as command line arguments... curl-O and curl-s comes to mind. And combos: curl-sv ...

I'd keep it simple. Expanding it would require changing the parser to bundle single character options but not allow long options, and would be more work to document for the users. Or add a different parser for this case. You end up with a mystery restriction - "you can append single character options that don't take a value to the command name when you softlink it". Or go down the rabbit hole of something like "_ means space in this case. And (on some platforms) hit filename length restrictions... I don't think it's worth the effort. There are other ways for a user to alias the other common options.

I think this case is the only one where you need the answer before parsing the command line. And the code need only check for argc >=1 && strlen(argv[0] >=2 && !strcmp(argv[0]+strlen(argv[0])-2,"-v") to set verbose/command trace mode. About 1 line of code. (Yes, you could factor out the strlen value for clarity and make it 2 lines of code...a compiler shouldn't care.)

@tlhackque
Copy link

Meant to add, specifically you can already do:

alias "curl-O=curl -O"
alias "curl-s=curl -s "

just like the the very common:

alias rm='rm -i'

Don't need a special mechanism for those cases.

@tlhackque
Copy link

And one other note:

If the default .curlrc can't be opened, it's OK to ignore it. The file is optional.

However, if the environment variable CURL_RC is defined but the file it points to can't be opened, it merits an error message. In this case, the user has requested a specific file, and would be surprised if it isn't used.

Should also note that checking the program name can be a little more involved on platforms that provide a file type (extension) as part of the name in argv[0]. In POSIX mode, most don't. But you may hit a case where, e.g. the program name is reported as curl-v.exe. So be prepared to strip a platform-dependent string from the end of the program name before looking for -v.

@vszakats
Copy link
Member

vszakats commented Apr 5, 2024

I hate to be the one coming up with this, but command aliasing on Windows is still not a universally (or at all?) usable feature. Symlinking used to require admin credentials, which might have changed recently, but this seems far from being a "natural" feature over there. Copying to another name works, but I'd vote for something not depending on argv[0].

(That said, this can be something curl supports, just not something to rely on exclusively.)

@tlhackque
Copy link

I hate to be the one coming up with this, but command aliasing on Windows is still not a universally (or at all?) usable feature. Symlinking used to require admin credentials, which might have changed recently, but this seems far from being a "natural" feature over there. Copying to another name works, but I'd vote for something not depending on argv[0].

(That said, this can be something curl supports, just not something to rely on exclusively.)

mklink will create softlinks on windows. Can be run with admin privs on any version of Windows. With windows 10, can also run as a regular user if "developer mode" is enabled. With win 11, a regular user can always create a symlink. (First useful change I know of in Win11...)

The symlink can be created as part of installing curl, so this isn't a significant limitation.

Of course, under cygwin or WSL, you get the POSIX behaviors.

VMS has had softlinks for a long time (and hard links forever), but may have the .EXE file extension.

While not perfect, all other solutions are (considerably) worse...

@vszakats
Copy link
Member

vszakats commented Apr 5, 2024

mklink will create softlinks on windows. Can be run with admin privs on any version of Windows. With windows 10, can also run as a regular user if "developer mode" is enabled. With win 11, a regular user can always create a symlink. (First useful change I know of in Win11...)

It was long overdue indeed.

We promise compatibility with Windows XP. We don't explicitly say if this is for running curl or building/testing curl, so it's fair to assume it means both.

Of course, under cygwin or WSL, you get the POSIX behaviors.

I don't use Windows, but this will be a problem for many I feel.

@tlhackque
Copy link

We promise compatibility with Windows XP. We don't explicitly say if this is for running curl or building/testing curl, so it's fair to assume it means both.

Of course, under cygwin or WSL, you get the POSIX behaviors.

I don't use Windows, but this will be a problem for many I feel.

As I said, curl can add the softlink link in the installer, and the option will be available from then on. Machines back to XP probably have all accounts as administrators anyway.

And note that this is an entirely new feature. It doesn't take anything away from a user who can't install as administrator.

No one has a current dependency on the feature. And on XP, it's quite possible to simply copy curl.exe to curl-v.exe if you're stuck with the need and without admin privileges.

Yes, one could also have a "CURL_VERBOSE=true" environment variable for this corner case. It's not that much extra code, especially if all it does is enable --verbose as opposed to unique "configuration debugging" as proposed. But it is one more thing to document and test. I don't think it's worth it; simple is better. But Daniel is doing the work...it's up to him to decide.

@dfandrich
Copy link
Contributor

dfandrich commented Apr 5, 2024 via email

@tlhackque
Copy link

. As others have said, there are already multiple ways for people to accomplish this without any changes needed to curl.

As Daniel said in introducing this, some change to curl is necessary in order to be able to selectively enable init file tracing.

Whether it's an environment variable, based on the program name, or some sort of pre-parsing the command line is what's being discussed.

Under my suggestion of using argv[0], nothing "breaks". Nor are you prevented from having multiple versions of curl installed.

There is simply a softlinked (or hard-linked, or renamed copy) of curl in the same place as the curl executable. If you have multiple curl executables, you have (optionally) a corresponding softlink for each.

As I've said, this is quite common. One common example is view, which is one of several softlink aliases for vi (today, most likely Vim, but this practice long pre-dates Vim).

ls -l /bin/view
lrwxrwxrwx 1 root root 2 Nov 28  2022 /bin/view -> vi

man vi
[...]
       Vim behaves differently, depending on the name of the command (the executable may still be the same file).

       vim       The "normal" way, everything is default.

       ex        Start  in  Ex  mode.  Go to Normal mode with the ":vi" command.  Can also be done with the "-e" argu-
                 ment.

       view      Start in read-only mode.  You will be protected from writing the files.  Can also be  done  with  the
                 "-R" argument.

       gvim gview
                 The GUI version.  Starts a new window.  Can also be done with the "-g" argument.

       evim eview
                 The GUI version in easy mode.  Starts a new window.  Can also be done with the "-y" argument.

       rvim rview rgvim rgview
                 Like  the  above, but with restrictions.  It will not be possible to start shell commands, or suspend
                 Vim.  Can also be done with the "-Z" argument.

@dfandrich
Copy link
Contributor

dfandrich commented Apr 5, 2024 via email

@tlhackque
Copy link

f I've compiled a special version of curl that I use for, let's say verification tasks and (whatever that is) and call it curl-v, then with the argv[0] proposal that curl will work differently than if I called it v-curl.

Yes, if you really did that, it breaks. How likely is that in real life?

Most people that I know don't put '-' in their aliases. curlv or vcurl are more likely than curl-v. But point taken.

You can quibble over what to call it - I think curl-v is mnemonic and easy to remember. Pick anything else. Doesn't have to have a '-'. verbose_curl? run_curl_verbosely? Or is this a philosophical objection rather than a pragmatic one?

What if someone has a script that has a CURL_RC environment variable (prior to this PR)? Say she uses it for "curl release candidate" rather than to specify an alternate to .curlrc? Is it OK to "break" that? I think it is - the curl project owns the curl namespace. In both cases.

What's your alternative? I've pointed out the difficulties with Daniel's baseline.

I think checking argv[0] is better than the alternatives.

But you folks can decide what you want to do. I've made my observations and constructive suggestions. I'm not inclined to extend this conversation.

@dfandrich
Copy link
Contributor

dfandrich commented Apr 5, 2024 via email

@vszakats
Copy link
Member

vszakats commented Apr 5, 2024

Even if we disregard XP and other environments without symlinks, I'm strongly on the opinion that no curl feature should exclusively depend on an OS feature requiring administrator rights and/or an installer. Such requirements can make life anything but simple.

Is there any issue with using an environment variable for this?

@tlhackque
Copy link

if it is to treat anything after a dash as an option, it would be carnage.

Ruled out early on. (.4 and .5) I believe this is the only case where curl needs information before parsing. The only other case I've encountered in real life is the related situation where you want to select a particular section of an init file that is read before processing the rest of the command line. But that doesn't apply here.

I'm skeptical about the "hundreds of people" who your hypothesize have invaded the curl namespace and picked curl-v as their special name.

@jay
Copy link
Member

jay commented Apr 5, 2024

I really dislike making debug output dependent on the position of the -v option because it's confusing and not intuitive. IMO it's too late for -vv since that would be unexpected by users that specify options multiple times.

CURL_CONFIG_DEBUG=1 curl ... seems like the best way

CURL_RC set to a (full) file name to read by default instead of $HOME/.curlrc

I think you're overdoing it here. Why do we need that?

@bagder
Copy link
Member Author

bagder commented Apr 8, 2024

CURL_RC set to a (full) file name to read by default instead of $HOME/.curlrc

I think you're overdoing it here. Why do we need that?

This would for example be an easy way to for example prevent ~/.curlrc to get loaded in a specific setup (like in the curl test suite). Or an easy way to pass on specific options to a range of curl command lines (like in a script).

@bagder
Copy link
Member Author

bagder commented Apr 8, 2024

I don't think we need to build in any specific behavior based on argv[0]. Aliases and scripts can already provide just about all that power if wanted.

@github-actions github-actions bot added the tests label Apr 15, 2024
@jay
Copy link
Member

jay commented Apr 16, 2024

CURL_RC set to a (full) file name to read by default instead of $HOME/.curlrc

I think you're overdoing it here. Why do we need that?

This would for example be an easy way to for example prevent ~/.curlrc to get loaded in a specific setup (like in the curl test suite). Or an easy way to pass on specific options to a range of curl command lines (like in a script).

I don't think it will be used properly. It could lead to breakage when users with good intentions override the default curlrc and then at some point scripts start to fail in mysterious ways. But, I seem to be alone on this :( OTOH I think #13387 is appropriate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants