Update documentation and port build system to CMake by aaron-sierra · Pull Request #5 · andy-shev/iotools

aaron-sierra · 2024-10-24T15:29:44Z

These changes include:

expanding the output of iotools --help and the project README.
providing a cross-build compatible alternative to iotools --make-links during install.
porting the build system to CMake for future dependencies and more trivial integration into Buildroot's package build framework.

Buildroot package reference files

Config.in

config BR2_PACKAGE_IOTOOLS
        bool "iotools"
        help
          The iotools package provides a set of simple command line
          tools which allow access to hardware device registers.
          Supported register interfaces include PCI, IO, memory mapped
          IO, SMBus, CPUID, and MSR. Also included are some utilities
          which allow for simple arithmetic, logical, and other
          operations.

          https://github.com/aaron-sierra/iotools/

if BR2_PACKAGE_IOTOOLS

config BR2_PACKAGE_IOTOOLS_NOLINKS
        bool "Exclude symlinks"
        default n

endif

iotools.mk

###############################################################################
#
# iotools
#
###############################################################################

IOTOOLS_VERSION = 3e0166d9cc832544279f27688279d6060c81b4b0
IOTOOLS_SITE = $(call github,aaron-sierra,iotools,$(IOTOOLS_VERSION))
IOTOOLS_LICENSE = GPLv2
IOTOOLS_LICENSE_FILES = COPYING
IOTOOLS_CONF_OPTS = -DSTATIC=OFF

ifeq ($(BR2_PACKAGE_IOTOOLS_NOLINKS),y)
IOTOOLS_CONF_OPTS += -DINSTALL_LINKS=OFF
endif

$(eval $(cmake-package))

andy-shev

Thanks for your hard work on this! It's almost good, some comments are spread around the commits.

andy-shev · 2024-10-31T17:14:54Z

 endif()

 option(STATIC "Build statically-linked iotools?" ON)
+option(INSTALL_LINKS "Generate symlinks during install?" ON)


Does it mean the default is on? I would move it OFF in this case (yes I know that initially it was like this, but again, I don't think distros will like this to be the default and I believe many of them will want to simply turn it OFF).

Yes, that default preserves this project's original view of symlinks as the primary means to access internal commands, because this work is merely a port from pure make to cmake.

I understand that your position is for the benefit of the downstream packager, but now that the packager can easily choose their preference, is it really worth changing the project's default without more concrete evidence of a global preference? If you still want to change the default, that should be done as an explicit commit after the cmake port has landed.

andy-shev · 2024-10-31T17:19:15Z

+
+# Install symlinks á la `iotools --make-links`, but in a cross-build
+# compatible manner.
+if (INSTALL_LINKS)


Cool, interesting trick!
The question here is how is it portable? And what if readelf is not available (may this be the case)?

The LLVM and binutils projects provide readelf implementations that are compatible with my proposal. Those are essential components of their respective build systems, so it should be safe to assume it will be available at build-time.

Make it readily available to the user that iotools supports three distinct modes of operation and create a clear distinction between options and commands. Usage: iotools COMMAND [ARGUMENTS] or: iotools OPTION or: COMMAND [ARGUMENTS] OPTIONS: --make-links create command symlinks and exit --list-cmds list available commands and exit --help print this message and exit -v, --version print version and exit COMMANDS: See output of --list-cmds Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Users requiring remote installation using SCP/SSH should use the new rinstall BASH script, instead. $ ./rinstall Usage: [RUSER=] RHOST= ./rinstall IOTOOLS_PATH [DESTDIR] Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Add a section (briefly) describing interaction with the binary. Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Calling --make-links assumes that iotools can be expected to run on the host architecture, despite the project intending to support native and cross-compilation equally well. Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

The DEBUG make variable is replaced by CMAKE_BUILD_TYPE, which defaults to Debug if undefined. The Debug and Release CFLAGS are aligned with the original Makefile, all others use CMake defaults. The project's initial configuration continues to enable static-linking of iotools by default, unless overridden by "cmake -D STATIC=0". A new configurable header, config.h.in, is added to tie VER_MAJOR and VER_MINOR to the implicit PACKAGE_VERSION_MAJOR and PACKAGE_VERSION_MINOR variables. Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

From https://cmake.org/cmake/help/latest/policy/CMP0065.html: CMake 3.3 and below, for historical reasons, always linked executables on some platforms with flags like -rdynamic to export symbols from the executables for use by any plugins they may load via dlopen(). CMake 3.4 and above prefer to do this only for executables that are explicitly marked with the ENABLE_EXPORTS target property. The OLD behavior of this policy is to always use the additional link flags when linking executables regardless of the value of the ENABLE_EXPORTS target property. The NEW behavior of this policy is to only use the additional link flags when linking executables if the ENABLE_EXPORTS target property is set to True. Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Add an INSTALL_LINKS configuration option to emulate the runtime --make-links option. It leverages a new and automatically-populated .tag field in struct cmd_info for easy lookup using readelf. The CMD_PREFIX define is set from the build script, so that the lookup mechanism has a good chance of staying in sync. Embedding the escaped code directly into CMakeLists allows us to use CMAKE_READELF, which isn't available from external scripts. This mechanism has been tested with readelf and llvm-readelf. Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

andy-shev

I'm going to apply the first part for now and then we can review the second one, there are still unanswered questions as far as I can see.

andy-shev · 2024-11-05T16:54:28Z

I'm going to apply the first part for now and then we can review the second one, there are still unanswered questions as far as I can see.

Ah, I can't do this. Please, split this to two PRs.

This is the first half of #5.

This is the first half of #5. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

aaron-sierra

These comments have been sitting "Pending" in a review on my side, because I failed to submit them. Sorry for the delay.

aaron-sierra · 2024-11-04T17:14:55Z

+
+# Install symlinks á la `iotools --make-links`, but in a cross-build
+# compatible manner.
+if (INSTALL_LINKS)


The LLVM and binutils projects provide readelf implementations that are compatible with my proposal. Those are essential components of their respective build systems, so it should be safe to assume it will be available at build-time.

aaron-sierra · 2024-11-04T17:25:36Z

 endif()

 option(STATIC "Build statically-linked iotools?" ON)
+option(INSTALL_LINKS "Generate symlinks during install?" ON)


Yes, that default preserves this project's original view of symlinks as the primary means to access internal commands, because this work is merely a port from pure make to cmake.

I understand that your position is for the benefit of the downstream packager, but now that the packager can easily choose their preference, is it really worth changing the project's default without more concrete evidence of a global preference? If you still want to change the default, that should be done as an explicit commit after the cmake port has landed.

andy-shev · 2024-11-11T20:39:52Z

These comments have been sitting "Pending" in a review on my side, because I failed to submit them. Sorry for the delay.

Interestingly that I can't reply in place there. So here are my answers:

Even if readelf is provided by main toolchains, can we actually test for it as we do for many other things in whatever buildsystem we have (CMake for this case)? Then we will be sure that it's present and we will know the exact path to it.
I got it, so, can we rather first to change the default and then update to CMake? I really don't like the tools like xor to be populated on the real filessystems, it's prone to collisions sooner or later. Same for any other names. If you want make this work, then it's probably should be installed in a separated folder (like Git does) and user can add it to the $PATH to be able to use. At least this solution will be much more cleaner (and I would not prevent the links to be created as a part of the build process).

Also a good to have is a variant of bash_completion (and alike to zsh, etc) to help with typing the commands.

aaron-sierra · 2024-11-14T02:38:07Z

I like the idea of shell completion as a future feature. I also think that reorganization of the link install location or other execution refactoring could reasonably occur in the future, too. At this time, I would like to preserve functionality that I (and former coworkers) have relied on for years.

My proposal relies on CMAKE_READELF, which is automatically discovered by CMake since 3.16.0 (2019) via Modules/CMakeFindBinUtils.cmake. That doesn't cover down to 3.0.0 (2014) like I'd intended, so I'm setting up version compatibility testing and have marked Port build system to CMake (2/2 from #5) #7 as Draft. Do you have issues with that discovery mechanism, assuming it can be backported?
How about I drop the 2.0.0 version bump commit, you land the pull request (after I address number 1 above), you push a commit making the default change, then you push a commit setting the 2.0.0 version?

andy-shev · 2024-11-20T17:30:40Z

I like the idea of shell completion as a future feature.

Yes, I'm not talking about fulfilling it right now.

I also think that reorganization of the link install location or other execution refactoring could reasonably occur in the future, too. At this time, I would like to preserve functionality that I (and former coworkers) have relied on for years.

Okay.

1. My proposal relies on `CMAKE_READELF`, which is automatically discovered by CMake since 3.16.0 (2019) via Modules/CMakeFindBinUtils.cmake. That doesn't cover down to 3.0.0 (2014) like I'd intended, so I'm setting up version compatibility testing and have marked [Port build system to CMake (2/2 from #5) #7](https://github.com/andy-shev/iotools/pull/7) as Draft. Do you have issues with that discovery mechanism, assuming it can be backported?

2014 is 10 years ago. I think we may rely on the feature, but make sure we require minimum CMake version.

2. How about I drop the 2.0.0 version bump commit, you land the pull request (after I address number **1** above), you push a commit making the default change, then you push a commit setting the 2.0.0 version?

Sounds like a plan!

The second part of the iotools update by Aaron, here is the conversion to use CMake.

This is the first half of #5.

The second part of the iotools update by Aaron, here is the conversion to use CMake.

This is a redo for #7, requested in #7 (comment).

aaron-sierra force-pushed the docs-and-build branch from e589d28 to 3e0166d Compare October 24, 2024 20:00

andy-shev requested changes Oct 31, 2024

View reviewed changes

Aaron Sierra added 11 commits November 4, 2024 12:17

build: Drop rinstall make target

17db2e9

Users requiring remote installation using SCP/SSH should use the new rinstall BASH script, instead. $ ./rinstall Usage: [RUSER=] RHOST= ./rinstall IOTOOLS_PATH [DESTDIR] Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

README: Fix long line in About section

d9a146f

Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

README: Convert to Markdown format

8ebf312

Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

README: Add Usage section

3007d1e

Add a section (briefly) describing interaction with the binary. Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Bump version to 1.8

953d4ac

Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

build: Do not call --make-links during install

4d10e1c

Calling --make-links assumes that iotools can be expected to run on the host architecture, despite the project intending to support native and cross-compilation equally well. Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

Bump version to 2.0

a706de3

Signed-off-by: Aaron Sierra <aaron@bubbl-tek.com>

aaron-sierra force-pushed the docs-and-build branch from 3e0166d to a706de3 Compare November 4, 2024 18:26

andy-shev approved these changes Nov 5, 2024

View reviewed changes

andy-shev closed this Nov 5, 2024

This was referenced Nov 5, 2024

Expand README and output of iotools --help (1/2 from #5) #6

Merged

Port build system to CMake (2/2 from #5) #7

Merged

andy-shev added a commit that referenced this pull request Nov 6, 2024

Expand README and output of iotools --help (1/2 from #5) (#6)

7545cbd

This is the first half of #5.

andy-shev added a commit that referenced this pull request Nov 6, 2024

Expand README and output of iotools --help (1/2 from #5) (#6)

3abdb69

This is the first half of #5. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

aaron-sierra commented Nov 11, 2024

View reviewed changes

andy-shev added a commit that referenced this pull request Jan 16, 2025

Port build system to CMake (2/2 from #5) (#7)

ddb8a6f

The second part of the iotools update by Aaron, here is the conversion to use CMake.

andy-shev added a commit that referenced this pull request Jan 16, 2025

Expand README and output of iotools --help (1/2 from #5) (#6)

f692f4a

This is the first half of #5.

andy-shev added a commit that referenced this pull request Jan 16, 2025

Port build system to CMake (2/2 from #5) (#7)

46d46af

The second part of the iotools update by Aaron, here is the conversion to use CMake.

andy-shev added a commit that referenced this pull request Jan 20, 2025

Port build system to CMake (2/2 from #5) (#8)

e27cb7c

This is a redo for #7, requested in #7 (comment).

Conversation

aaron-sierra commented Oct 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Buildroot package reference files

Config.in

iotools.mk

Uh oh!

andy-shev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andy-shev Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

aaron-sierra Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

andy-shev Oct 31, 2024

Choose a reason for hiding this comment

Uh oh!

aaron-sierra Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

andy-shev left a comment

Choose a reason for hiding this comment

Uh oh!

andy-shev commented Nov 5, 2024

Uh oh!

aaron-sierra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aaron-sierra Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

aaron-sierra Nov 4, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andy-shev commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaron-sierra commented Nov 14, 2024

Uh oh!

andy-shev commented Nov 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aaron-sierra commented Oct 24, 2024 •

edited

Loading

andy-shev commented Nov 11, 2024 •

edited

Loading