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

Support options like --config --part and --port #1922

Open
MCUdude opened this issue Sep 8, 2024 · 11 comments · May be fixed by #1936
Open

Support options like --config --part and --port #1922

MCUdude opened this issue Sep 8, 2024 · 11 comments · May be fixed by #1936
Labels
enhancement New feature or request

Comments

@MCUdude
Copy link
Collaborator

MCUdude commented Sep 8, 2024

I created a post over at the Avrfreaks forum announcing that Avrdude v8.0 has just been released. The feedback was mixed (some pointing out that Avrdude is just a CLI tool and not a full-blown GUI, and thus not relevant in 2024), but @WestfW mentioned that the option flags (-C, -c, -p etc) could be difficult to remember, and having full-blown --flags would be nice.

Avrdude currently uses getopt, and this doesn't support --flags. Instead, we'll have to use argp.

Are there any downsides to using agrp instead of getopt?

@MCUdude MCUdude added the enhancement New feature or request label Sep 8, 2024
@ndim
Copy link
Contributor

ndim commented Sep 8, 2024

In PR #1698, I have replaced the getopt call with getopt_long in order to add --version.

While at it, I also added--help.

Quoting the FreeBSD man page for getopt_long:

HISTORY
     The getopt_long() and getopt_long_only() functions first appeared in the
     GNU libiberty library.  The first BSD implementation of getopt_long()
     appeared in NetBSD 1.5, the first BSD implementation of
     getopt_long_only() in OpenBSD 3.3.  FreeBSD first included getopt_long()
     in FreeBSD 5.0, getopt_long_only() in FreeBSD 5.2.

We already have a Windows implementation of getopt_long inside src/msvc/getopt.c, so changing from getopt to getopt_long is very quick.

So getopt_long is covered on all systems with glibc i.e. Linux and compatible (e.g. musl), BSD (and therefore MacOS), and Windows.

The argp API is a lot more powerful, of course, but it appears avrdude would need to ship an argp implementation for all non-glibc platforms in order to use the argp API with its advantages like autogenerated --help output.

@ndim
Copy link
Contributor

ndim commented Sep 8, 2024

To actually answer the question: The downside to using argp is the work needed to ensure its availability on all systems other than Linux/glibc, both from a technical and from a licensing point of view.

@WestfW
Copy link

WestfW commented Sep 9, 2024

getopt_long() seems to drop in pretty painlessly, and is already present in the windows getopt used by avrdude, and works on both macos and linux builds as well, so it should be pretty safe.

(from the avrfreaks discussion):
I replaced in implicit include for getopt() from unistd.h with an explicit include of getopt.h (which is already present in src/msvc/)

Then it was just a matter of replacing the getopt() call with one to getopt_long() (and adding the structure with the long form commands...)

Everything else stayed the same!

 // Process command line arguments
 static struct option long_options[] =
   {
     { "baud",      required_argument, 0, 'b' },
     { "bitclock",  required_argument, 0, 'B'},
     { "programmer", required_argument, 0, 'c'},
     { "config",    required_argument, 0, 'C'},
     { "noerase",   no_argument,      0, 'D'},
     { "erase",     no_argument,      0, 'e'},
     { "logfile",   required_argument, 0, 'l'},
     { "test",      no_argument,      0, 'n'},
     { "noconfig",  no_argument,      0, 'N'},
     { "part",      required_argument, 0, 'p'},
     { "chip",      required_argument, 0, 'p'},
     { "port",      required_argument, 0, 'P'},
     { "quiet",     no_argument,      0, 'q'},
     { "reconnect", no_argument,      0, 'r'},
     { "terminal",  no_argument,      0, 't'},
     { "tty",       no_argument,      0, 't'},
     { "memory",    required_argument, 0, 'U'},
     { "verbose",   no_argument,      0, 'v'},
     { "noverify",  no_argument,      0, 'V'},
     {0, 0, 0, 0}
   };
 int long_index;
 
 while((ch = getopt_long(argc, argv,
                         "?Ab:B:c:C:DeE:Fi:l:nNp:OP:qrtT:U:vVx:",
                         long_options, &long_index)) != -1) {
   switch(ch) {

That does leave a bunch of help text and error messages that contain only the short command forms...

@stefanrueger
Copy link
Collaborator

@ndim Thanks for above comment about getopt_long(). I assume that supersedes your previous comment expressing getopt_long() might not be portable to all OSes we are currently supporting? If you think we'd be OK with getopt_long() and @WestfW's table above would you be interested in extending --version and --help along above table? That would be cool. The main outstanding work will be updating help texts in main.c and updating the man and tex documentation.

@ndim
Copy link
Contributor

ndim commented Nov 21, 2024

Both --part and --chip mapping to p makes sense to me, but @WestfW your long_options struct contains two long options terminal and tty which both map to the same character t. Is that on purpose?

ndim added a commit to ndim/avrdude that referenced this issue Nov 21, 2024
Switch from getopt(3) to getopt_long(3) and add a few long options
as aliases to existing short options, e.g. "--help" and "--part"
being aliases for "-?" and "-p", respectively.

The getopt_long(3) function is available on GNU, BSD, and the existing
msvc/getopt.[ch] already implements getopt_long() on Windows. This
should cover all systems avrdude supports.

Adapt the avrdude usage message shown by "-?" or "--help" to show the
new long options.

TODO: Adapt man page and texi manual reflecting the long options.

Closes: avrdudes#1922
@stefanrueger
Copy link
Collaborator

I am guessing(!) tty ought to map tp P. Also, on closer reflextion, perhaps increase coverage, ie, invent a long version for every option letter?

@ndim
Copy link
Contributor

ndim commented Nov 23, 2024

FWIW, I have examined the argp implementation inside gnulib with a view towards possible redistribution inside the avrdude repo, and for linking into the avrdude command line tool. The argp features would be very nice to use, after all.

The gnulib argp implementation appears to be licensed LGPL-3.0-or-later and GPL-3.0-or-later and originate from the GNU libc. There are already two GPL-3 (without "or-later") source files in the avrdude repo (avrintel.c and libavrdude-avrintel.h) alongside many GPL-2.0-or-later files, so linking GPL-3.0-or-later sources into libavrdude or avrdude would not change their respective binary's (library or CLI tool) GPL-3.0 terms, AFAICT.

[user@home ~/avrdude]$ tabs -30
[user@home ~/avrdude]$ licensecheck --machine --shortname-scheme=spdx src/*.[ch]
src/arduino.c                 GPL-2.0-or-later
src/arduino.h                 GPL-2.0-or-later
src/avr910.c                  GPL-2.0-or-later
...
src/whereami.h                WTFPL-2
src/wiring.c                  GPL-2.0-or-later
src/wiring.h                  GPL-2.0-or-later
src/xbee.c                    GPL-2.0-or-later
src/xbee.h                    GPL-2.0-or-later
[user@home ~/avrdude]$ tabs -8

I have not yet verified that the gnulib argp implementation would actually build for and run on all of avrdude's supported operating systems. If avrdude were to switch to argp, glibc on Linux systems would obviously work, but non-glibc Linux systems, the BSDs, MacOS, and Windows still need to be examined.

@stefanrueger Is the GPL-3.0 only licensing for avrintel.c and libavrdude-avrintel.h on purpose? If yes, the repo should ship that license text, as the built libavrdude and avrdude will then fall under GPL-3.0, not the GPL-2.0-or-later which the COPYING file describes and most *.[ch] files use.

If the move to GPL-3.0 is on purpose, I can check whether we could use gnulib's argp implementation.

If we can relicense the avrintel.c and libavrdude-avrintel.h files to something compatible with GPL-2.0-or-later and avrdude and libavrdude is supposed to remain GPL-2.0-or-later, then continuing to invest time into argp would not make sense, as argp's LGPL-3.0-or-later together with avrdude's GPL-2.0-or-later would result in a GPL-3.0-or-later binary.

ndim added a commit to ndim/avrdude that referenced this issue Nov 23, 2024
Switch from getopt(3) to getopt_long(3) and add a few long options
as aliases to existing short options, e.g. "--help" and "--part"
being aliases for "-?" and "-p", respectively.

The getopt_long(3) function is available on GNU, BSD, and the existing
msvc/getopt.[ch] already implements getopt_long() on Windows. This
should cover all systems avrdude supports.

Adapt the avrdude usage message shown by "-?" or "--help" to show the
new long options.

TODO: Adapt man page and texi manual reflecting the long options.

Closes: avrdudes#1922
@stefanrueger
Copy link
Collaborator

stefanrueger commented Nov 23, 2024

Is the GPL-3.0 only licensing for avrintel.c and libavrdude-avrintel.h on purpose?

No: I've just used a licence that I was using for other projects. I am happy to put another licence on these files that fits avrdude better, so happy to change to GPL-2.0-or-later.

Wrt to argp I'd argue that this decision should be more driven by how much we need its features. I tend to go with a philosophy of using what we cannot do without rather than using what we could do with. And yes, if we don't really need argp's unique features then it's better to keep avrdude at gpl-2.0-or-later as (in theory) more projects that interested to use avrdude will be able to cope with that.

@MCUdude
Copy link
Collaborator Author

MCUdude commented Nov 23, 2024

I think getopt_long would be sufficient. The only place where support for "multiple words" would be neat would be with the -U flag.

However, it would be a big improvement if it would be possible to support something like this, and print a sensible error message if the user got the -U / --memory syntax wrong:

-U flash:write:myfile.hex
-U flash:read:mydump.hex
-U flash:verify:myfile.hex

--memory flash:write:myfile.hex
--memory flash:read:mydump.hex
--memory flash:verify:myfile.hex

--memory flash:w:myfile.hex
--memory flash:r:mydump.hex
--memory flash:v:myfile.hex

@ndim
Copy link
Contributor

ndim commented Nov 23, 2024

Just had an idea: It might be possible to find a LGPL-2.0-or-later argp implementation in older versions of gnulib.

Here we have the lib/argp*.[ch] files as GPL-2.0-or-later

Source repo: git://git.savannah.gnu.org/gnulib.git
git hash:    b42d50108cf2147b3d80ecb6457faa96347411e9
Date:        Sun Oct 7 17:38:16 2007 +0200

but making them compile without copying successively more files from gnulib is non-trivial (and I do not know where this would end). Also, there might have been important fixes between 2007-10-07 and now...

Summarizing: For avrdude, I do prefer getopt_long over argp_parse now.

@stefanrueger
Copy link
Collaborator

Extending the -U syntax to allow flash:write:myfile.hex might be somewhat involved (currently there are only one-letter memory operations r, w and v foreseen in the syntax). Also sensible error messages might be hard to come by as : is both an admissible file character in Windows and a syntax separator for -U, so avrdude can never be quite sure which form of memory command was intended if it is somehow malformed. There is

  • -U filename
  • -U memory:op:filename
  • -U memory:op:filename:encoding

It helps that op and encoding are only ever one character.

So to cut a long short, I recommend keeping this issue at the scope of long options and documenting them.

ndim added a commit to ndim/avrdude that referenced this issue Nov 24, 2024
Switch from getopt(3) to getopt_long(3) and add a few long options
as aliases to existing short options, e.g. "--help" and "--part"
being aliases for "-?" and "-p", respectively.

The getopt_long(3) function is available on GNU, BSD, and the existing
msvc/getopt.[ch] already implements getopt_long() on Windows. This
should cover all systems avrdude supports.

Adapt the avrdude usage message shown by "-?" or "--help" to show the
new long options.

TODO: Adapt man page and texi manual reflecting the long options.

Closes: avrdudes#1922
ndim added a commit to ndim/avrdude that referenced this issue Nov 27, 2024
Switch from getopt(3) to getopt_long(3) and add a few long options
as aliases to existing short options, e.g. "--help" and "--part"
being aliases for "-?" and "-p", respectively.

The getopt_long(3) function is available on GNU, BSD, and the existing
msvc/getopt.[ch] already implements getopt_long() on Windows. This
should cover all systems avrdude supports.

Adapt the avrdude usage message shown by "-?" or "--help" to show the
new long options.

TODO: Adapt man page and texi manual reflecting the long options.

Closes: avrdudes#1922
@ndim ndim linked a pull request Nov 27, 2024 that will close this issue
6 tasks
ndim added a commit to ndim/avrdude that referenced this issue Nov 27, 2024
Switch the command line argument parser from getopt(3) to
getopt_long(3) and add a few long options as aliases to existing
short options, e.g. "--help" and "--part" being aliases for "-?"
and "-p", respectively.

The getopt_long(3) function is available on GNU, BSD, and the existing
msvc/getopt.[ch] already implements getopt_long() on Windows. This
should cover all systems avrdude supports.

Adapt the avrdude usage message shown by "-?" or "--help" to show the
new long options.

TODO: Adapt man page and texi manual reflecting the long options.

Closes: avrdudes#1922
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants