avrdudes / avrdude

AVRDUDE is a utility to program AVR microcontrollers
GNU General Public License v2.0
650 stars 124 forks source link

Avrdude always needs the absolute path to avrdude.conf #770

Closed kristofmulier closed 2 years ago

kristofmulier commented 2 years ago

Whenever one launches avrdude, it seems like the full absolute path to the avrdude.conf file must be given with the -C argument. I tried with a relative path (relative to the avrdude executable, something like ../etc/avrdude.conf) but that doesn't work. Avrdude absolutely wants the absolute path.

In the makefile that we created for Embeetle IDE, we therefore end up with monstruous expressions, like:

FLASHTOOL_PATH = $(if $(filter $(dir .),$(dir $(FLASHTOOL))),$(firstword $(wildcard $(patsubst %,%/$(FLASHTOOL),$(subst $(PATHSEP), ,$(PATH))))),$(FLASHTOOL))

.PHONY: flash
flash: $(BINARIES) print_flash
    "$(FLASHTOOL)" -C "$(subst /bin/../,/,$(subst /avrdude\$,,$(FLASHTOOL_PATH)\$)/../)etc/avrdude.conf" \
                 -v \
                 -patmega328p \
                 -carduino \
                 -P$(FLASH_PORT) \
                 -b115200 \
                 -D \
                 -Uflash:w:$(ELF_FILE:.elf=.hex):i

It would be so much easier if one could pass a relative path in the -C argument. Or even if the -C argument could be left out alltogether and avrdude looks itself for the file at ../etc/avrdude.conf.

dl8dtl commented 2 years ago

No, it's not necessarily an absolute path.

There is a system default directory where it is being searched for, ususally ${prefix}/etc/ (where prefix could be like /usr, or /usr/local). Only if it's not found there, the -C option is required. The location of this directory can be overridden at ./configure using the --sysconf-dir option. The idea behind all this is, for a self-consistent standard installation, there is never a reason to provide a -C option at all.

Deriving the location of the binary itself (in order to walk from there to the system configuration directory) is a non-trivial task. If the program was started by the surrounding shell by searching along ${PATH}, argv[0] does not contain a directory component but just "avrdude" only. Under unixoid systems, it's not even guaranteed the binary itself still has a directory entry while it is running: if someone deleted the file right after starting it, the directory entry will be gone yet the physical file keeps existing as long as it is being executed (deletion will proceed once it exited).

Btw., if you specify a relative path (without a leading slash), it is simply searched relative to the current working directory it has been started from.

mariusgreuel commented 2 years ago

@kristofmulier: Could you provide some background information as to why you need the -C option? Do you ship a local copy of avrdude? Or do you use a custom config with the system supplied binary?

dl8dtl commented 2 years ago

Btw., instead of a custom copy, it's always preferrable to use a ~/.avrduderc file instead to put local additions into. The regular avrdude.conf is supposed to belong into AVRDUDE's domain, to be replaceable at the next version upgrade.

kristofmulier commented 2 years ago

Hi @dl8dtl and @mariusgreuel , Apologies for my late reply.

1. Context

I'm working on a new, free microcontroller IDE (see https://embeetle.com). In our IDE you can manage your tools in the home window:

Here users can point to an existing avrdude installation on their computer. However, if the user has none, our IDE offers a download. The user can then decide where to put the downloaded tool. The default is somewhere in the IDE installation folder. Typically it would end up at:

<embeetle_installation_folder>/beetle_tools/avrdude_6.3.0_32b

The executable would then be at:

<embeetle_installation_folder>/beetle_tools/avrdude_6.3.0_32b/bin/avrdude.exe

and the config file at:

<embeetle_installation_folder>/beetle_tools/avrdude_6.3.0_32b/etc/avrdude.conf

 

2. How Embeetle IDE deals with tools

We have a not-so-common philosophy when it comes to dealing with tools. Our IDE never "installs" a tool on your PC (tools like compiler toolchains, flash tools (openocd, avrdude), GNU Make, ...). Instead, the IDE just drops a downloaded tool somewhere on your harddrive (always giving you a choice where) and that's it. No messing with your $PATH, certainly no messing with your registry (on Windows), no addition of files in your users folder - nothing.

The advantage is that your PC remains clean. If you ever delete our IDE (and the tools therein), you don't have to clean up anything else.

There are of course a few downsides. Let me explain.

If you run a microcontroller project and you click the build or flash button, the corresponding makefile target launches:

image

For the record: all microcontroller projects in our IDE are (at the moment) makefile-based. If the user writes his own makefile and puts his own tools on the $PATH, it will just work that way. Most users however rely on our sample projects that carry our default makefile. In this makefile, we cannot rely on the $PATH to find tools. So this makefile expects a few parameters to hold the absolute paths to the tools, such as $(COMPILER_TOOLCHAIN), $(FLASHTOOL), ...

In other words - Embeetle IDE always passes these parameters when invoking the makefile.

 

3. How our default makefile deals with avrdude

As said before, our default makefile cannot rely on the $PATH, nor can it rely on a configuration file that could be or could not be present in a location like ~/.avrduderc. The only things our default makefile can rely on:

  1. It gets the absolute path to the avrdude executable as a parameter when the makefile is invoked. More in particular, as parameter $(FLASHTOOL).
  2. All avrdude installations I have seen so far have a default config file at ../etc/avrdude.conf relative to the executable.

So this is how we construct the flash target in the default makefile (slightly simplified):

.PHONY: flash
flash:
    "$(FLASHTOOL)" -C "$(subst /bin/../,/,$(subst /avrdude\$,,$(FLASHTOOL)\$)/../)etc/avrdude.conf" \
                       -v \
                       -patmega328p \
                       -carduino \
                       -P$(FLASH_PORT) \
                       -b115200 \
                       -D \
                       -Uflash:w:$(ELF_FILE:.elf=.hex):i

As you can see, we do provide a -C parameter to launch avrdude, such that it can find the configuration file close to the executable. It only works on Linux where the $(FLASHTOOL) parameter has no .exe suffix. The .exe suffix somehow messes up the expression. We didn't yet come up with a solution for that.

Another issue is that we cannot always rely on $(FLASHTOOL) holding the absolute path to avrdude. Yes, if it runs in Embeetle, it will always be the absolute path. But at the top of our makefile, we do provide a fallback value:

FLASHTOOL = avrdude

This fallback value is there for those users who run the makefile outside the IDE and don't pass absolute toolpath parameters when invoking it.

We came up with a monstrous contraption to solve this. The following variable is created to hold always the absolute path:

FLASHTOOL_PATH = $(if $(filter $(dir .),$(dir $(FLASHTOOL))),$(firstword $(wildcard $(patsubst %,%/$(FLASHTOOL),$(subst $(PATHSEP), ,$(PATH))))),$(FLASHTOOL))

Then it is used in the invocation of avrdude:

.PHONY: flash
flash:
    "$(FLASHTOOL)" -C "$(subst /bin/../,/,$(subst /avrdude\$,,$(FLASHTOOL_PATH)\$)/../)etc/avrdude.conf" \
                       -v \
                       -patmega328p \
                       -carduino \
                       -P$(FLASH_PORT) \
                       -b115200 \
                       -D \
                       -Uflash:w:$(ELF_FILE:.elf=.hex):i

It kinda works (aside from the .exe suffix for Windows that we didn't solve yet), but it's just plain ugly. We also target the educational sector. Giving them makefiles like this won't make them happy.

 

4. Solutions

I think there are several routes to go from here:

  1. We come up with a solution for the .exe suffix for Windows. Our makefile expressions become even more monstruous.

  2. Avrdude gains the ability to find the configuration file at ../etc/avrdude.conf all by itself. Of course, this assumes that avrdude is aware of its own location. I understand from @dl8dtl that this is non-trivial. Is there really no solution to this?

  3. Could the data in the default ../etc/avrdude.conf file also be present in the executable itself? This way, avrdude always has a fallback in case no config file is found. On the downside, it would make the executable a few hundred kilobytes larger. On the plus-side, it would make avrdude much more user friendly for people who didn't setup a config file anywhere.

Thank you so much for your help :-)

dl8dtl commented 2 years ago

I absolutely don't like the idea of embedding an entire avrdude.conf into the binary itself. This is prone to misunderstandings, and user frustration ("I did change that value in the file, why doesn't it pick it up?"). If your target is Windows-only, the avrdude.conf location is derived differently from Unix right now, see src/confwin.c. I understand it's searched along $PATH though which is not going to help you either … What might be possible is to see whether AVRDUDE has been called with an absolute path name, and first look from there in ../etc for the system-wide configuration file. Failing that, we then fall back to the sysconf-dir provided at build time. Alas, path name (separator) handling on Windows is messy, since you have to consider both \ and /, and there might be a "drive" name in front of it or not. You even have to consider a mixture like C:\Program Files\avrdude/bin/avrdude. (On unixoid systems, it's way easier, as there's only the forward slash.) If you'd like to have that feature, I'd kindly ask you to provide a sample implementation, and make a pull request for it. Please don't forget to also update the documentation (at least one of both, either the man page avrdude.1, or the texinfo file avrdude.texi), as the new config file lookup behaviour needs to be documented to users.

kristofmulier commented 2 years ago

Hi @dl8dtl , Thank you very much for your quick reply!

I absolutely don't like the idea of embedding an entire avrdude.conf into the binary itself. This is prone to misunderstandings, and user frustration ("I did change that value in the file, why doesn't it pick it up?").

Agreed. One way to combat such misunderstandings is to explicitely mention in the avrdude output that no config file was found and the built-in fallback is used. Of course, I will not push for such a solution if it makes you feel uncomfortable.

What might be possible is to see whether AVRDUDE has been called with an absolute path name, and first look from there in ../etc for the system-wide configuration file [..]

That would be great!

If you'd like to have that feature, I'd kindly ask you to provide a sample implementation, and make a pull request for it [..]

I would love to, but I have no idea how the AVRDUDE source code works. Making myself comfortable with the source code will take an eternity. At the same time, I don't want to abuse the goodness from people like you, under the pretext of open source. Please let me know how to make a donation to the avrdude project.

Would you mind to add me on LinkedIn? https://www.linkedin.com/in/kristof-mulier-686919109/

Or Facebook: https://www.facebook.com/kristof.mulier

Hope we can connect ^_^

mariusgreuel commented 2 years ago

As far as Windows is concerned, the configuration file is located via SearchPath(), which picks up the .conf file in the directory of the binary itself (among others, essentially its DLL search order). So putting the .conf file into the same folder as the binary does what you need. For Un*x, the path is hardcoded to the preferred sysconf path. While I agree that for local installations, it would be nice if AVRDUDE could locate the conf file automatically, a quick search confirms Jörgs assessment on finding the process binaries path: https://stackoverflow.com/a/1024937

I think you should -C only if you use the Embeetle installation of AVRDUDE. Perhaps it is possible to pass in the root path of the Embeetle installation to the makefile, so you can derive the bin and conf files path from that? Otherwise, you would just omit the -C option and hope for a working system installation.

dl8dtl commented 2 years ago

I would love to, but I have no idea how the AVRDUDE source code works.

That part is pretty simple:

#if defined(WIN32NATIVE)

  win_sys_config_set(sys_config);
  win_usr_config_set(usr_config);

#else

  strcpy(sys_config, CONFIG_DIR);
  i = strlen(sys_config);
  if (i && (sys_config[i-1] != '/'))
    strcat(sys_config, "/");
  strcat(sys_config, "avrdude.conf");

That CONFIG_DIR is the mentioned sysconfig-dir from the configuration script. You'd need to extend the above logic to find out whether argv[0] is an absolute path. The logic is straight inside of main() (in src/main.c), and it is called before getopt(), so you do have access to argv[0] in its original form. You don't have to understand the remainder of AVRDUDE in order to modify that part. You are the one who could best be able to test any sample implementation, and see whether it would meet your needs.

ps: Sorry, I'm neither on LinkedIn nor Facebook.

kristofmulier commented 2 years ago

As far as Windows is concerned, the configuration file is located via SearchPath(), which picks up the .conf file in the directory of the binary itself (among others, essentially its DLL search order). So putting the .conf file into the same folder as the binary does what you need.

Awesome! This solves everything for the Windows case.

For Unx, the path is hardcoded to the preferred sysconf path. While I agree that for local installations, it would be nice if AVRDUDE could locate the conf file automatically, a quick search confirms Jörgs assessment on finding the process binaries path:* https://stackoverflow.com/a/1024937

So there are several ways for AVRDUDE to figure out its own location on Unix systems, all of which have their own drawbacks. Do you suggest to stick to argv[0]? Or would you suggest another way?

You'd need to extend the above logic to find out whether argv[0] is an absolute path. The logic is straight inside of main() (in src/main.c), and it is called before getopt(), so you do have access to argv[0] in its original form. You don't have to understand the remainder of AVRDUDE in order to modify that part.

I'll give it a try ^_^

Sorry, I'm neither on LinkedIn nor Facebook.

My mail is (email omitted) Hope to connect through mail :-)

@mariusgreuel, do you have Facebook or LinkedIn?

Thanks @dl8dtl and @mariusgreuel for your help!

dl8dtl commented 2 years ago

Do you suggest to stick to argv[0]?

Yes, please. I wouldn't want to drag in a truckload of system-dependant #ifdefs for just that simple purpose. Use argv[0]. If you only want to do that on unixoid systems, it is pretty simple anyway: watch out whether argv[0][0] is a slash. If not, you don't have an absolute path name, and just go ahead using the pre-configured directory. (That is not as generic as the solutions on that website, e.g. if you run ../../avrdude, it won't work, but I guess that's not an issue in your case.) If it is a slash, then you have that internal variable progname which is basically a pointer to the trailing program name. So say, you have

argv[0] = "/usr/bin/avrdude";
                    ^--- progname points here (hmm, this might not be displayed correctly)

You could then copy everything from argv[0] up to progname into a new buffer, append ../etc/ to it, and look whether you find an avrdude.conf there, by just trying to open it. If you get an ENONENT, revert to the pre-configured path instead.

kristofmulier commented 2 years ago

Thanks a lot @dl8dtl ! I will first try to just clone the AVRDUDE git repo and build it. Then I'll take small steps towards adding this feature. I'll keep you in the loop :-)

kristofmulier commented 2 years ago

Hi @dl8dtl and @mariusgreuel , I have modified the code in main.c and will describe the modifications first here before launching a pull request.

The code I focused on starts at line 420:

#if defined(WIN32NATIVE)

  win_sys_config_set(sys_config);
  win_usr_config_set(usr_config);

#else

  strcpy(sys_config, CONFIG_DIR);
  i = strlen(sys_config);
  if (i && (sys_config[i-1] != '/'))
    strcat(sys_config, "/");
  strcat(sys_config, "avrdude.conf");

  ...

#endif

I stayed out of the Windows block. After all, the Windows case is solved by just putting avrdude.conf in the same folder with the executable. I can do that for the build we distribute from our server. Case closed.

For the Linux block, I made the following change:

#else
  /*
   * Let sys_config point to the local config file, such as
   * "/usr/local/etc/avrdude.conf"
   */
  strcpy(sys_config, CONFIG_DIR);
  i = strlen(sys_config);
  if (i && (sys_config[i-1] != '/'))
    strcat(sys_config, "/");
  strcat(sys_config, "avrdude.conf");

  /*
   * In case there is no config file, try to locate the config file near the
   * executable. First verify if there is a config file in the same folder with
   * the executable. Then look in "../etc/".
   */
  if(access(sys_config, F_OK) != 0){
    // First locate the absolute path to the avrdude executable
    char avrdude_abspath[PATH_MAX];
    if (strlen(argv[0]) && (argv[0][0] == '/')) {
      // argv[0] holds the absolute path to avrdude
      strcpy(avrdude_abspath, argv[0]);
      avrdude_abspath[PATH_MAX-1] = 0;
    }
    else {
      // Try getenv("_"), which works in most Unix shells
      if(getenv("_") && strlen(getenv("_")) && (getenv("_")[0] == '/')) {
        strcpy(avrdude_abspath, getenv("_"));
        avrdude_abspath[PATH_MAX-1] = 0;
      }
    }

    // If the absolute path to the avrdude executable was found, try to locate
    // the config file from there.
    if(strlen(avrdude_abspath) && (avrdude_abspath[0] == '/')) {
      // First look in the same folder with the executable
      strcpy(sys_config, dirname(strdup(avrdude_abspath)));
      sys_config[PATH_MAX-1] = 0;
      i = strlen(sys_config);
      if (i && (sys_config[i-1] != '/'))
        strcat(sys_config, "/");
      strcat(sys_config, "avrdude.conf");
      sys_config[PATH_MAX-1] = 0;

      // Next look in the "../etc/" folder
      if(access(sys_config, F_OK) != 0){
        strcpy(sys_config, dirname(dirname(strdup(avrdude_abspath))));
        sys_config[PATH_MAX-1] = 0;
        i = strlen(sys_config);
        if (i && (sys_config[i-1] != '/'))
          strcat(sys_config, "/");
        strcat(sys_config, "etc/avrdude.conf");
        sys_config[PATH_MAX-1] = 0;
      }
    }
  }
  // avrdude_message(MSG_INFO, "sys_config = %s\n\n", sys_config);

  ...

#endif

Note: Because I'm using the dirname() function, I also added the #include <libgen.h> statement at line 36.

First I check if the sys_config variable (which has a value like /usr/local/etc/avrdude.conf) points at an existing file. If there is no file there, I start the procedure to locate a config file near the executable.

The procedure starts with the creation of the avrdude_abspath string variable. It gets the value of argv[0] if argv[0][0] == '/'. Otherwise, it gets the value of getenv("_"). Most Unix shells store the absolute path to the executable in that variable.

If variable avrdude_abspath was successfully assigned an absolute path(1), I go on to the next step. I look into the folder of the executable for avrdude.conf. If not there, I look into the ../etc folder for the file. The config file location is then written to sys_config. Task completed!

 
(1) To check if avrdude_abspath was successfully assigned the absolute path, I just
verify if the first character of the string is a '/'. Probably there must be better solutions.

dl8dtl commented 2 years ago

Kristof, thank you very much for coming up with a proposal. I wonder whether the search order shouldn't rather be reversed: first, in <path of executable>/../etc, then in the hard-configured location. That would allow users to install a separate AVRDUDE version in a directory that is not the one it has been configured at compile-time, and prefer that version's avrdude.conf over the system one. My shell (tcsh) doesn't set "_" in environment, nor does my /bin/sh, so that "most shells" is probably an overstatement. bash does, so does zsh. ksh does it in a different way, it writes something like *1644*/path/to/binary where the number between the stars is the PID of the shell itself … not sure whether it's really worth trying to handle that "_" variable. I'd say, if argv[0] starts with a slash, use it as an absolute path name, otherwise assume we don't know it.

strdup'ing something (which causes the duplicate to be malloc'ed) and then not caring about deallocation of the returned string is bad practice. Also, strdup() can potentially yield NULL (from malloc()) so passing it straight into dirname() is certainly not a good idea. All in all, I wouldn't perform any directory traversal myself: if you have the absolute path name, search for the rightmost '/', and assume everyone up to that being the directory name. Just append ../etc/avrdude.conf, and try to open that as the configuration file. (I'm not sure whether there are any unix-like installations that have the config file in the same directory as the executable, that's not common practice on unix systems.)

dl8dtl commented 2 years ago

But, you can really feel free to actually implement your suggestion, and make a pull request out of it. That way, you can give it a try first whether it satisfies your needs, and it's still easy enough for us to even make inline comments in the respective code lines upon review.

kristofmulier commented 2 years ago

Kristof, thank you very much for coming up with a proposal.

My pleasure 😊

I wonder whether the search order shouldn't rather be reversed: first, in <path of executable>/../etc, then in the hard-configured location. That would allow users to install a separate AVRDUDE version in a directory that is not the one it has been configured at compile-time, and prefer that version's avrdude.conf over the system one.

Good idea. So the code where CONFIG_DIR is used, should then come last?

[..] not sure whether it's really worth trying to handle that "_" variable. I'd say, if argv[0] starts with a slash, use it as an absolute path name, otherwise assume we don't know it.

The problem is that argv[0] only contains the absolute path if AVRDUDE was invoked with an absolute path. If the user would add AVRDUDE to his $PATH environment variable and then invoke our makefile outside Embeetle IDE, it would no longer work. In that case, AVRDUDE would simply be invoked as:

$ avrdude

And argv[0] just contains the string "avrdude".

Using the "_" variable as a fallback won't solve the issue completely, but will at least make it work for many users (bash shell is super popular). It's better than nothing.

The post at https://stackoverflow.com/a/1024937 shows a few more solutions to solve the issue. Maybe I should dive into some of them. Unfortunately, they all look a bit complicated.

strdup'ing something (which causes the duplicate to be malloc'ed) and then not caring about deallocation of the returned string is bad practice [..]

My apologies. I've actually never written C code for a desktop application before. Only embedded C for small microcontrollers that have no heap. So I'm not used to the malloc and dealloc stuff. I'll try to improve my code.

All in all, I wouldn't perform any directory traversal myself: if you have the absolute path name, search for the rightmost '/', and assume everyone up to that being the directory name. Just append ../etc/avrdude.conf, and try to open that as the configuration file.

Good suggestion. I'll do that. This way, I can also leave out the extra #include statement.

But, you can really feel free to actually implement your suggestion, and make a pull request out of it.

I built my code (together with all of AVRDUDE of course) and tried it on my PC. It did what I needed. However, I will improve my code based on the suggestions you made above. Then I will file a pull request and we can continue to fine tune the code such that it fits the high standards of AVRDUDE 😊👍

Thanks a lot @dl8dtl . I thank you for your patience with me, and your mentoring. 😊

kristofmulier commented 2 years ago

Hi @dl8dtl and @mariusgreuel , I'm thinking about using the whereai library: https://github.com/gpakosz/whereami/tree/master/src

It's an MIT licensed library, consisting of just two files:

According to the library author, the two files just need to be dropped in the project. It works on these platforms:

What do you think?

dl8dtl commented 2 years ago

Since PR #780 has been integrated, this can be closed now.