Skip to content

Conversation

@EchterAgo
Copy link
Contributor

@EchterAgo EchterAgo commented Oct 4, 2023

The code to detect the available drivers does so by parsing Makefile.am but it does not do so relative to "$srcdir" so trying to do a configure outside of the source directory will fail.

General points

  • Described the changes in the PR submission or a separate issue, e.g.
    known published or discovered protocols, applicable hardware (expected
    compatible and actually tested/developed against), limitations, etc.

  • There may be multiple commits in the PR, aligned and commented with
    a functional change. Notably, coding style changes better belong in a
    separate PR, but certainly in a dedicated commit to simplify reviews
    of "real" changes in the other commits. Similarly for typo fixes in
    comments or text documents.

Frequent "underwater rocks" for driver addition/update PRs

  • Revised existing driver families and added a sub-driver if applicable
    (nutdrv_qx, usbhid-ups...) or added a brand new driver in the other
    case.

  • Did not extend obsoleted drivers with new hardware support features
    (notably blazer and other single-device family drivers for Qx protocols,
    except the new nutdrv_qx which should cover them all).

  • For updated existing device drivers, bumped the DRIVER_VERSION macro
    or its equivalent.

  • For USB devices (HID or not), revised that the driver uses unique
    VID/PID combinations, or raised discussions when this is not the case
    (several vendors do use same interface chips for unrelated protocols).

  • For new USB devices, built and committed the changes for the
    scripts/upower/95-upower-hid.hwdb file

  • Proposed NUT data mapping is aligned with existing docs/nut-names.txt
    file. If the device exposes useful data points not listed in the file, the
    experimental.* namespace can be used as documented there, and discussion
    should be raised on the NUT Developers mailing list to standardize the new
    concept.

  • Updated data/driver.list.in if applicable (new tested device info)

Frequent "underwater rocks" for general C code PRs

  • Did not "blindly assume" default integer type sizes and value ranges,
    structure layout and alignment in memory, endianness (layout of bytes and
    bits in memory for multi-byte numeric types), or use of generic int where
    language or libraries dictate the use of size_t (or ssize_t sometimes).
  • Progress and errors are handled with upsdebugx(), upslogx(),
    fatalx() and related methods, not with direct printf() or exit().
    Similarly, NUT helpers are used for error-checked memory allocation and
    string operations (except where customized error handling is needed,
    such as unlocking device ports, etc.)

  • Coding style (including whitespace for indentations) follows precedent
    in the code of the file, and examples/guide in docs/developers.txt file.

  • For newly added files, the Makefile.am recipes were updated and the
    make distcheck target passes.

General documentation updates

  • Updated docs/acknowledgements.txt (for vendor-backed device support)

  • Added or updated manual page information in docs/man/*.txt files
    and corresponding recipe lists in docs/man/Makefile.am for new pages

  • Passed make spellcheck, updated spell-checking dictionary in the
    docs/nut.dict file if needed (did not remove any words -- the make
    rule printout in case of changes suggests how to maintain it).

Additional work may be needed after posting this PR

  • Propose a PR for NUT DDL with detailed device data dumps from tests
    against real hardware (the more models, the better).

  • Address NUT CI farm build failures for the PR: testing on numerous
    platforms and toolkits can expose issues not seen on just one system.

  • Revise suggestions from LGTM.COM analysis about "new issues" with
    the changed codebase.

The code to detect the available drivers does so by parsing
`Makefile.am` but it does not do so relative to "$srcdir" so trying
to do a configure outside of the source directory will fail.

Signed-off-by: Axel Gembe <[email protected]>
@EchterAgo EchterAgo marked this pull request as ready for review October 4, 2023 09:06
@EchterAgo
Copy link
Contributor Author

I use this with VSCode and the following tasks.json to have multiple builds from the same source tree:

{
    // See https://go.microsoft.com/fwlink/?LinkId=733558
    // for the documentation about the tasks.json format
    "version": "2.0.0",
    "tasks": [
        {
            "label": "autogen",
            "type": "process",
            "command": "/bin/sh",
            "args": [ "${workspaceFolder}/autogen.sh" ],
            "options": {
                "cwd": "${workspaceFolder}"
            },
            "problemMatcher": []
        },
        {
            "label": "configure",
            "type": "process",
            "command": "/bin/sh",
            "args": [
                "${workspaceFolder}/configure",
                "--with-drivers=usbhid-ups,apc_modbus",
                "--with-usb",
                "--prefix=/home/ago/src/nut/prefix",
                "--with-modbus",
                "--with-modbus-includes=-I/home/ago/src/libmodbus/prefix/include/modbus",
                "--with-modbus-libs=-L/home/ago/src/libmodbus/prefix/lib -lmodbus",
                "--enable-warnings",
                "--enable-spellcheck",
                "CPPFLAGS=-DDEBUG",
                "CFLAGS=-g -O0"
            ],
            "options": {
                "cwd": "${workspaceFolder}/build/dev"
            },
            "dependsOn": ["autogen"],
            "problemMatcher": []
        },
        {
            "label": "configure debian",
            "type": "process",
            "command": "/bin/sh",
            "args": [
                "${workspaceFolder}/configure",
                "--with-drivers=usbhid-ups,apc_modbus",
                "--with-usb",
                "--prefix=",
                "--sysconfdir=/etc/nut",
                "--with-statepath=/run/nut",
                "--with-user=nut",
                "--with-group=nut",
                "--with-modbus",
                "--with-modbus-includes=-I/home/ago/src/libmodbus/prefix/include/modbus",
                "--with-modbus-libs=-L/home/ago/src/libmodbus/prefix/lib -lmodbus"
            ],
            "options": {
                "cwd": "${workspaceFolder}/build/debian"
            },
            "dependsOn": ["autogen"],
            "problemMatcher": []
        },
        {
            "label": "configure distcheck",
            "type": "process",
            "command": "/bin/sh",
            "args": [
                "${workspaceFolder}/configure",
                "--with-usb",
                "--with-modbus",
                "--with-modbus-includes=-I/home/ago/src/libmodbus/prefix/include/modbus",
                "--with-modbus-libs=-L/home/ago/src/libmodbus/prefix/lib -lmodbus"
            ],
            "options": {
                "cwd": "${workspaceFolder}/build/distcheck"
            },
            "dependsOn": ["autogen"],
            "problemMatcher": []
        },
        {
            "label": "make",
            "type": "process",
            "command": "make",
            "args": [ "-j", "64" ],
            "problemMatcher": [ "$gcc" ],
            "options": {
                "cwd": "${workspaceFolder}/build/dev"
            },
            "group": {
                "kind": "build",
                "isDefault": true
            }
        },
        {
            "label": "make debian",
            "type": "process",
            "command": "make",
            "args": [ "-j", "64" ],
            "problemMatcher": [ "$gcc" ],
            "options": {
                "cwd": "${workspaceFolder}/build/debian"
            },
            "group": {
                "kind": "build",
                "isDefault": false
            }
        },
        {
            "label": "make distcheck",
            "type": "process",
            "command": "make",
            "args": [ "-j", "64", "distcheck" ],
            "problemMatcher": [ "$gcc" ],
            "options": {
                "cwd": "${workspaceFolder}/build/distcheck"
            },
            "group": {
                "kind": "build",
                "isDefault": false
            }
        }
    ]
}

@EchterAgo
Copy link
Contributor Author

Note that this is only needed when selecting drivers via the --with-drivers configure option. To test this, you can create a directory outside of the nut source directory, or even a subdirectory of the nut source directory and then run /path/to/nut/configure --with-drivers=usbhid-ups. With this patch it will succeed and give you a directory you can run make in, without this patch it will give you:

checking which drivers to build... usbhid-ups
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against NUTSW_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against SERIAL_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against USB_LIBUSB_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against SERIAL_USB_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against SNMP_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against NEONXML_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against MACOSX_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against MODBUS_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against LINUX_I2C_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against POWERMAN_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against IPMI_DRIVERLIST=""
../../configure: line 17502: drivers/Makefile.am: No such file or directory
configure: Will check custom driver selection against GPIO_DRIVERLIST=""
configure: error: Requested driver 'usbhid-ups' is not defined in drivers/Makefile.am (or configure.ac has a bug/lag detecting driver lists)

@jimklimov jimklimov added bug packaging CI Entries related to continuous integration infrastructure (here CI = tools + scripts + recipes) portability We want NUT to build and run everywhere possible labels Oct 4, 2023
@jimklimov jimklimov added this to the 2.8.1 milestone Oct 4, 2023
@jimklimov
Copy link
Member

Thanks!

@jimklimov jimklimov merged commit 5ebc699 into networkupstools:master Oct 4, 2023
@EchterAgo EchterAgo deleted the autoconf_out_of_tree_build branch October 4, 2023 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug CI Entries related to continuous integration infrastructure (here CI = tools + scripts + recipes) packaging portability We want NUT to build and run everywhere possible

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants