caprice32 icon indicating copy to clipboard operation
caprice32 copied to clipboard

Finding config file: inconsistency between doc and behavior

Open cpcitor opened this issue 4 years ago • 4 comments

Inconsistency between doc and behavior

Documentation says:

Caprice32 will try and open, in this order: $CWD/cap32.cfg ($CWD being the directory where the cap32 executable resides), then a .cap32.cfg file in the user home directory, then /etc/cap32.cfg.

Comment in cap32.h confirms:

// Return the path to the best (i.e: most specific) configuration file.
// Priority order is:
//  - cap32.cfg in the same directory as cap32 binary
//  - $HOME/.cap32.cfg
//  - /etc/cap32.cfg
std::string getConfigurationFilename(bool forWrite = false);

But code does:

   if(getcwd(chAppPath, sizeof(chAppPath)-1) == nullptr) { // get the location of the executable

Which does not get the location of the executable but the current directory of the process that run cap32.

Sanity check:

cd /tmp ; strace -etrace=file /path/to/cap32  2>&1 | grep cfg

access("/tmp/cap32.cfg", F_OK)             = -1 ENOENT (No such file or directory)
access("/home/stephane/.cap32.cfg", F_OK) = 0

See, it does not look into /path/to/cap2.cfg but /tmp/cap32.cfg.

While this is a possible policy, this contradicts the documentation.

How this is traditionally done

Traditionally, at source configuration time, a, install prefix is provided (typically /usr, plus sysconfigdir at etc), or a default used (typically /usr/local or /opt). That information is compiled in the binary, so that the binary know where to look for.

Non-conformity to the DRY principle

Makefile and configuration are written with the assumption that the program will be installed to /usr/local. That information is repeated:

cap32.cfg:53:resources_path=/usr/local/share/caprice32/resources cap32.cfg:170:rom_path=/usr/local/share/caprice32/rom makefile:53:prefix = /usr/local

Not much of a hell of a repetition, but as a result, people integrating caprice32 (whether in a distribution or in a package like cpc-dev-tool-chain) have to adjust for it, typically by patching, e.g. debian/patches/fix-install-path.patch, which still has some conditions that prevent to work out-of-the-box (for one, finding the patched cfg file).

Suggestion

  • Do nothing. People can generate cap32 files and use a wrapper script automatically instruct cap32 about where config is, see e.g. https://github.com/cpcitor/cpc-dev-tool-chain/blob/7a8b03babda7d87c95e5b8337be77f3d82066307/tool/caprice32/bin/cap32 .
  • directly honour PREFIX and SYSCONFIGDIR from compile time.
  • switch to autoconf to do that. No don't do that. You'd then have two problems. https://softwareengineering.stackexchange.com/questions/223634/what-is-meant-by-now-you-have-two-problems
  • switch to CMake. Not enough of a reason to be worth it. Would have the benefit to replace the main Makefile, to the cost of learning enough of CMake and having it as build time dependency.
  • Do nothing. People can patch your source and offer PRs. ;-)

cpcitor avatar Dec 11 '19 08:12 cpcitor

The cleaner way to solve the second "problem" is probably to go for option 2. Honor PREFIX and SYSCONFIGDIR from compile time. Since (God forbids !) we do not use autoconf & friends, I guess it would mean scripting around to modify the source before compilation.

For the first one, I guess the simplest way would be to look at the path used to invoke cap32 and derive the actual dir from here… assuming a sadistic user did not play with symlinks.

Your thought on this ? I can write a couple of patch to handle this - this is Xmas vacation after all, so I have a bit of time !

sebhz avatar Dec 23 '19 20:12 sebhz

HI @sebhz !

I have checked again. I just found there is a pull request that was merged to master and addresses related issues.

It adds the possibility of writing this:

make APP_PATH="$PWD"

The generated binary will look for cap32.cfg first in the directory provided at APP_PATH.

So, is the bug fixed?

No, because following the README still leads to the same behavior: compiling then calling cap32 from another directory fails to find associated config file.

How to fix it then?

A bug happens when documentation (or the lack thereof) yields reasonable expectations and the observed behavior does not match the expectations. So we can either document current behavior or change it.

What about paths in cfg file?

In cap32.cfg, relative paths (e.g. rom) are still resolved relative to process current directory. This is okay when only absolute paths are used there.

When a cap32.cfg entry is empty, the program computes a default , taking app_path into account (see https://github.com/ColinPitrat/caprice32/blob/master/src/cap32.cpp#L1634 ). This works when program is in source tree.

My overall feeling is that this could be simplified: refactor common parts into smaller, simpler, more consistent elements. It feels like a common case of a project and makefile growing little by little.

So?

  • Generally, a "debug" setup (source tree, not installed anywhere) for an independent project like cap32 should never (or should it?) look for globally installed configuration, or even user-level config file.
  • I can work with current state, so there is little incentive for me to change it.
    • Latest PR removed need to call a wrapper script. Generating a cfg file with absolute path works.
    • Alternatively, if relocatability is wished and a wrapper script is acceptable, making a wrapper script that changes directory then calls cap32 with a cap32.cfg that contain relative paths in cap32.cfg should work.

The bottom line is: I'll live with this for now, yet if someone can pick up the idea and simplify the current code, this would be nice. Oh, this is not my project, @ColinPitrat is current maintainer, so he's the one to know anyway.

Thanks for the PR - at least now the doc is in line with the behavior. I know Colin is the maintainer of this project. He'll probably merge your PR shortly.

sebhz avatar Jan 02 '20 16:01 sebhz

I'm open to changes around this as long as it's in the direction of making things simpler while preserving:

  • a way to provide a config file on the command line (useful for tests, having parallel setups ...)
  • using the local configuration when developing (i.e naked make && ./cap32) to ease developers life
  • having a way to have a system configuration and a user configuration overriding it
  • using decent defaults when no configuration is found

ColinPitrat avatar Jan 08 '20 12:01 ColinPitrat