Skip to content

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

Merged
andy-shev merged 4 commits into
andy-shev:masterfrom
aaron-sierra:cmake-build
Jan 16, 2025
Merged

Port build system to CMake (2/2 from #5)#7
andy-shev merged 4 commits into
andy-shev:masterfrom
aaron-sierra:cmake-build

Conversation

@aaron-sierra
Copy link
Copy Markdown
Collaborator

This is the second half of #5 and, therefore, depends on #6 which ends with the version 1.8 bump.

These changes include:

  • 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))

Copy link
Copy Markdown
Owner

@andy-shev andy-shev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now you can drop the first part of the PR and rebase it against current master/main.

@andy-shev
Copy link
Copy Markdown
Owner

Also, please address the comments I have given in #5.

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>
@aaron-sierra
Copy link
Copy Markdown
Collaborator Author

Now you can drop the first part of the PR and rebase it against current master/main.

I've rebased to account for the merge commit.

@aaron-sierra
Copy link
Copy Markdown
Collaborator Author

Also, please address the comments I have given in #5.

If you are referring to these discussions, I await further comment from you:

@andy-shev
Copy link
Copy Markdown
Owner

Also, please address the comments I have given in #5.

If you are referring to these discussions, I await further comment from you:

Hmm... Maybe I don't understand something. There two comments from me and no replies from you. Can you elaborate what I should do / write?

@aaron-sierra
Copy link
Copy Markdown
Collaborator Author

Hmm... Maybe I don't understand something. There two comments from me and no replies from you.

I'd left my comments "Pending" 🤦 They should be visible to you, now.

@aaron-sierra aaron-sierra marked this pull request as draft November 14, 2024 02:14
@aaron-sierra aaron-sierra marked this pull request as ready for review December 11, 2024 22:50
@aaron-sierra
Copy link
Copy Markdown
Collaborator Author

The current version implements changes discussed here:

  • Set minimum CMake version to 3.16
  • Omit 2.0.0 bump

Copy link
Copy Markdown
Owner

@andy-shev andy-shev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments, but overall looks good to me.

Comment thread iotools.c
Comment thread CMakeLists.txt
Comment thread CMakeLists.txt
Comment thread CMakeLists.txt
Comment thread CMakeLists.txt
Aaron Sierra added 3 commits December 12, 2024 12:19
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 PROJECT_VERSION_MAJOR and
PROJECT_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 (circa CMake 3.16), 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>
@andy-shev andy-shev merged commit ddb8a6f into andy-shev:master Jan 16, 2025
andy-shev added a commit that referenced this pull request Jan 16, 2025
The second part of the iotools update by Aaron, here is the conversion to use CMake.
@andy-shev
Copy link
Copy Markdown
Owner

@aaron-sierra Sorry, I have screwed up something. Can you create a new PR based on what's now in the master branch?

The problem was that the repository has diverged in a sense of v1.8 tag and what's in the master. I synced it, but it produced two similar merge commits (of v1.8). So I reset the master to v1.8 to start over.

@aaron-sierra
Copy link
Copy Markdown
Collaborator Author

Hi Andy, I synced my fork to yours right after the merge occurred, so you should be able to pull ddb8a6f (#7 with merge commit) from there.

@andy-shev
Copy link
Copy Markdown
Owner

Hi Andy, I synced my fork to yours right after the merge occurred, so you should be able to pull ddb8a6f (#7 with merge commit) from there.

Nope that won't work, your repo has the same issue mine had. You need to be sure that you have commit 3abdb69 in your repo. For this you need to run git pull --rebase origin master and see what will happen.

@aaron-sierra
Copy link
Copy Markdown
Collaborator Author

I see, you amended 7545cbd with a sign-off, creating 3abdb69:

image

I tend to work with a fast-forward-only merge policy to avoid automatically generated commit messages, so I'm not entirely clear how my pull request was able to rewrite history in your master. Anyway, look for the new PR shortly.

andy-shev added a commit that referenced this pull request Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants