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

Add build information to libavrdude and avrdude and display it #1698

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ndim
Copy link
Contributor

@ndim ndim commented Feb 21, 2024

The goal of this Draft PR is to develop a way to add useful build information to libavrdude and avrdude, give libavrdude users (like avrdude) access to that buildinfo, and have avrdude show the buildinfo.

The ideas for this were originally discussed in #1681 (comment) and the following comments.

This is a basic proof of concept for showing a long verbose message with build information. The verbosity, the exact information, its representation, the API and ABI are subject to discussion.

At this time, the code is based on PR #1681.

This is quick and dirty and not tested comprehensively:

  • Switch avrdude's main.c from getopt() to getopt_long() (available on GNU, BSD, and
    the existing msvc/getopt.[ch] already implement getopt_long() on
    Windows)

  • Add --version argument which shows a long version message.

    As -V is already in use, the version message is available only with the long form --version.

  • Define build information for libavrdude and the avrdude tool

    • in buildinfo.c and libavrdude.h for libavrdude
    • in main.c for avrdude

The result looks about like

avrdude (...) 7.3-20240221 (28e6f254)
Copyright (C) ...
License GPL...
This is free software...
avrdude 7.3-20240221 (28e6f254)
  1. buildsystem: autotools
libavrdude 7.3-20240221 (28e6f254)
  1. buildsystem: autotools
  2. libelf
  3. libusb
  4. libusb_1_0
  5. libhidapi
  6. libftdi1
  7. libreadline
  8. libserialport
  9. parport
 10. linuxgpio
 11. linuxspi

@ndim
Copy link
Contributor Author

ndim commented Feb 21, 2024

Interestingly, the msvc x64 build fails when compiling buildinfo.c, because the included libavrdude.h uses PATH_MAX as an array size but PATH_MAX is not defined in the msvc x64 environment.

src/libavrdude.h Outdated
@@ -1522,8 +1526,8 @@ void terminal_setup_update_progress();
extern "C" {
#endif

void win_sys_config_set(char sys_config[PATH_MAX]);
void win_usr_config_set(char usr_config[PATH_MAX]);
void win_sys_config_set(char *sys_config);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well spotted!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After removing all uses of PATH_MAX from libavrdude.h, it might also be possible to remove the header file #include which used to pull in the header file defining PATH_MAX.

That will certainly improve portability.

src/libavrdude.h Outdated
@@ -866,7 +866,11 @@ typedef struct { // Memory cache for a subset of cached pages
#define OFF 0 // Many contexts: reset, power, LEDs, ...
#define ON 1 // Many contexts

#ifdef PATH_MAX
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have never been happy with the port[] array in the programmer structure, so I have now created PR #1699 that does away with this array and uses just a const char * pointer. As a consequence PGM_PORTLEN should no longer be needed once that PR is merged.

@mcuee mcuee added the enhancement New feature or request label Feb 22, 2024
const char *const *libavrdude_buildinfo = avr_get_buildinfo();
printf("libavrdude %s\n", libavrdude_buildinfo[0]);
print_buildinfos(libavrdude_buildinfo);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a good reason to use printf instead of msg_info here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a quick hack proof of concept, and I have not familiarized myself with the message API yet, so... No :-)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, take your time. For reference, here are all of them:

avrdude/src/avrdude.h

Lines 64 to 96 in 5c1649d

// Shortcuts
#define msg_ext_error(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_EXT_ERROR, __VA_ARGS__)
#define msg_error(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_ERROR, __VA_ARGS__)
#define msg_warning(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_WARNING, __VA_ARGS__)
#define msg_info(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_INFO, __VA_ARGS__)
#define msg_notice(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_NOTICE, __VA_ARGS__)
#define msg_notice2(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_NOTICE2, __VA_ARGS__)
#define msg_debug(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_DEBUG, __VA_ARGS__)
#define msg_trace(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_TRACE, __VA_ARGS__)
#define msg_trace2(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, 0, MSG_TRACE2, __VA_ARGS__)
#define pmsg_ext_error(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FUNCTION|MSG2_FILELINE|MSG2_TYPE|MSG2_FLUSH, MSG_EXT_ERROR, __VA_ARGS__)
#define pmsg_error(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FUNCTION|MSG2_FILELINE|MSG2_TYPE|MSG2_FLUSH, MSG_ERROR, __VA_ARGS__)
#define pmsg_warning(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FUNCTION|MSG2_FILELINE|MSG2_TYPE|MSG2_FLUSH, MSG_WARNING, __VA_ARGS__)
#define pmsg_info(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_INFO, __VA_ARGS__)
#define pmsg_notice(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_NOTICE, __VA_ARGS__)
#define pmsg_notice2(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_NOTICE2, __VA_ARGS__)
#define pmsg_debug(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_DEBUG, __VA_ARGS__)
#define pmsg_trace(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_TRACE, __VA_ARGS__)
#define pmsg_trace2(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_PROGNAME|MSG2_FLUSH, MSG_TRACE2, __VA_ARGS__)
#define imsg_ext_error(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT1|MSG2_FLUSH, MSG_EXT_ERROR, __VA_ARGS__)
#define imsg_error(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT1|MSG2_FLUSH, MSG_ERROR, __VA_ARGS__)
#define imsg_warning(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT1|MSG2_FLUSH, MSG_WARNING, __VA_ARGS__)
#define imsg_info(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT2|MSG2_FLUSH, MSG_INFO, __VA_ARGS__)
#define imsg_notice(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT2|MSG2_FLUSH, MSG_NOTICE, __VA_ARGS__)
#define imsg_notice2(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT2|MSG2_FLUSH, MSG_NOTICE2, __VA_ARGS__)
#define imsg_debug(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT2|MSG2_FLUSH, MSG_DEBUG, __VA_ARGS__)
#define imsg_trace(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT2|MSG2_FLUSH, MSG_TRACE, __VA_ARGS__)
#define imsg_trace2(...) avrdude_message2(stderr, __LINE__, __FILE__, __func__, MSG2_INDENT2|MSG2_FLUSH, MSG_TRACE2, __VA_ARGS__)
#define term_out(...) avrdude_message2(stdout, __LINE__, __FILE__, __func__, MSG2_FLUSH, MSG_INFO, __VA_ARGS__)
#define fmsg_out(fp, ...) avrdude_message2(fp, __LINE__, __FILE__, __func__, MSG2_FLUSH, MSG_INFO, __VA_ARGS__)

  • There are msg_* macros that just print whatever you insert.
  • There are pmsg_* that prints avrdude: and then whatever you insert
  • There are imsg_* that indents so that text printed on the screen will align with p_msg

And suffixes:

  • There are _info which prints what you insert regardless of mode
  • There are notice that will print with one or more -v's
  • There are notice2 that will print with two or more -v's
  • And more 🙂

@ndim ndim force-pushed the add-build-information branch 2 times, most recently from 0fa4d47 to f5bfe97 Compare February 25, 2024 23:10
@mcuee mcuee added bug Something isn't working and removed bug Something isn't working labels Feb 25, 2024
@ndim ndim force-pushed the add-build-information branch 2 times, most recently from f09ff55 to 2c53d4b Compare July 31, 2024 15:01
@ndim
Copy link
Contributor Author

ndim commented Jul 31, 2024

As build version information is system independent, build version (git branches, etc.) information it relatively simple to add.

Adding information about the optional libraries and features built into this version is a bit more complex. To do that properly, we need to build a bit of build infrastructure, and then make use of that:

  • cmake buildsystem function generate .c files containing build information for avrdude and libavrdude
  • automake buildsystem macro generate .c files containing build information for avrdude and libavrdude, possibly emulating the VERSIONINFO_SETUP to fetch that information from the cmake config files
  • build object file with build information and link that into the avrdude executable and libavrdude library
  • Add avrdude --version or something similar which shows all that verbose build information (note that avrdude does not support long form arguments until now)

Aiming for a quicker, "good enough" class of solution first, then the proper one.

@stefanrueger
Copy link
Collaborator

Same here: @ndim Just an advance warning that the team have started testing with a view to release v8.0 around the w/end 17/18 Aug. If you have time to bring this PR into a shape that we can merge even partial progress that would be brill (but no worries if not).

@stefanrueger
Copy link
Collaborator

BTW, I think the merge conflict comes about b/c the getopt() options were slightly changed: Y was deprecated and has no longer an optional argument, so the last : was removed....

@ndim
Copy link
Contributor Author

ndim commented Aug 6, 2024

Update: Issue solved already. I should read my own commit comments.

The biggest problem here is that my Linux getopt(3) man page says getopt() is part of POSIX.1-2008, while getopt_long() is GNU. And GNU functions do not necessarily work on BSD, OSX, and Windows.

@ndim
Copy link
Contributor Author

ndim commented Aug 6, 2024

Oh, and I want a different C API for the version information. The "list of strings" as used by libgphoto2 has drawbacks. I would prefer a "list of key-value-pairs" type of interface, with keys like "gpiod" and values like "<1.4" or ">=1.4<1.8" or ">= 1.8".

Such an API should be well thought out, as it should last at least 10 years. Not sure this should be rushed within the next 10 days.

@ndim ndim force-pushed the add-build-information branch 2 times, most recently from 2e08789 to fedf11b Compare August 7, 2024 01:32
@ndim ndim force-pushed the add-build-information branch 2 times, most recently from fdb0603 to ab7d9dc Compare August 24, 2024 21:18
Switch from getopt() to getopt_long() and add a long "--help" argument
as alias to the "-?" argument.

Note that the getopt_long() function is available on GNU, BSD, and the
existing msvc/getopt.[ch] already implements getopt_long() on Windows.
Add basic buildinfo ideas, quick and dirty and not tested comprehensively:

  * Define build information for libavrdude and the avrdude tool

  * Add a long "--version" argument which shows a long version message.

XXX Just add the output to "avrdude -v" header?
XXX Define the first (key, value) tuple to be
(package_name, package_version)?

If there is any extra information, we can always later add
a (key,value) tuple like e.g. ("git branch", "main")
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 this pull request may close these issues.

4 participants