Config file parsing bug?

General Discussion
1

Basic Information:

Printer Model: Voron 2.4
MCU / Printerboard: LDO Leviathan and Mellow SB2040
Host / SBC: Raspberry Pi 4b

Describe your issue:

It seems that one of the GitHub commits from 10/30/24 has changed the way that macros and/or include files are parsed. At the end of my printer.cfg file I have the following lines.

[exclude_object]
[include mainsail.cfg]
[include mainsail_modifications.cfg] # Must come after “[include mainsail.cfg]”
[include pause_resume.cfg] # Must come after “[include mainsail.cfg]”
[include bed_probe.cfg]
[include heat_soak.cfg]
[include bed_surfaces.cfg]
[include leds.cfg]
[include pressure_advance_calibration.cfg]
[include filament_detection.cfg]
[include filament.cfg]

The mainsail_modifications.cfg and pause_resume.cfg files are used to override some of the configuration from mainsail.cfg. The most up to date code from GitHub results in the mainsail_modifications.cfg and pause_resume.cfg files seemingly being ignored. Rolling back 7 commits fixes the problem and everything works the way it did before. This can be seen by comparing the two log files below. I am not sure if this is a bug or intended behavior, but I wanted to bring it to everyone’s attention since I did not see any changes to the documentation or anyone post anything about this so far.

New log file, rolled back log file, mainsail_modifications.cfg, and pause_resume.cfg files:
Logs and cfgs.zip (36.2 KB)

2

Pretty sure I saw a commit that disabled the include directive from working in sub-files. Do the files that stopped working contain [include] directives?

3

I saw that as well (commit 9d4ab86), but the files that stopped working do not contain any [include] directives and the only file where I use [include] is the printer.cfg file. However, the files that stopped working do contain config sections and macros like [pause_resume] and CANCEL_PRINT that are duplicates with the mainsail.cfg file. Previously, these duplicates would override the ones already defined in mainsail.cfg as long as the files were included after mainsail.cfg. Since the other files without duplicates work fine, I think the problem is specifically related to the duplicates.

4

You didn’t include your klippy.log (just excerpts with the useful information cut out) so it’s very hard to try to reproduce this.

5

Log file is below:
klippy.log (275.5 KB)

6

Thank you, that’s very helpful.

The issue is, as I suspected, the comments on those two lines, which before configfile: Only check for [include file] directives from main printe… · Klipper3d/klipper@9d4ab86 · GitHub Klipper would strip off before parsing out the filename. In combination with bpo-38741: Definition of multiple ']' in header configparser (GH-17129) · python/cpython@2924bb1 · GitHub this makes Klipper running under Python >= 3.11 treat the filename to be included as mainsail_modifications.cfg] # Must come after “[include mainsail.cfg; normally a nonexistent included file is an error but this string contains [ so it looks somewhat like a wildcard and the error is suppressed.

1 Like
7

Thanks, that makes sense. I moved the comments to a separate line and that resolved the problem.

I do think that this way of parsing [include] will probably result in similar issues in the future. Would it make sense to only allow one include directive per line and use the second ] to indicate the end of that include? That way any comments afterward would be ignored as usual.

8

The most correct fix would be to have Python parse the data instead of doing it manually, since it does so correctly under all versions.

from __future__ import print_function

import sys

try:
    import configparser
except ImportError:
    import io
    import ConfigParser as configparser

iclude = '[include foo] # not [include bar]'                                   

if sys.version_info.major >= 3:
    cp = configparser.RawConfigParser(strict=False, inline_comment_prefixes=(';', '#'))
    cp.read_string(iclude)
else:
    cp = configparser.RawConfigParser()
    buf = io.StringIO(unicode(iclude))
    cp.readfp(buf)

mo = configparser.RawConfigParser.SECTCRE.match(iclude)                        

print('Python: ', cp.sections())
print('Manual: ', mo.group('header'))

Python 2.7:

Python:  [u'include foo']
Manual:  include foo

Python 3.6:

Python:  ['include foo']
Manual:  include foo

Python 3.12:

Python:  ['include foo']
Manual:  include foo] # not [include bar
2 Likes
9

Thanks. It does seem like commit 9d4ab862 introduced a regression. It should hopefully be fixed now (commit f119e96e).

-Kevin

10

This topic was automatically closed 30 days after the last reply. New replies are no longer allowed.