Skip to content

Conversation

@Nathan-Mossaad
Copy link
Contributor

This PR builds off of sr/bench and introduces the following:

Features

  • Format checking GitHub workflow to check if formatting has been applied on every push/PR

Documentation

  • Updated build instructions in README (to use Nix environment)
  • Introduced docs for formatting with clang-format, cmake-format, and nixfmt

Formatting

  • Clang formatting applied to all .cpp and .h files
  • CMake formatting using clang-format
  • Nix formatting for flake.nix

Fixes

  • Excluded CMakeLists.txt in .gitignore
  • Added ever-changing guest-sections.lds to .gitignore

Cleanup

  • Removed unneeded guest-sections.lds (it gets regenerated on every execution of txlat)
  • Removed broken shell.nix

on the intended host architecture (e.g. Arancini for ARM64 must be built on ARM).

A typical build proceeds as follows:
A typical build proceeds as follows (currently nix is required):
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong, you can also use cmake by hand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback, but I do not agree with this statement.

In most distributions, the bundled version of fmt is much newer and provides a different API, whereby compiling fails (as there are no checks if the correct version of fmt is installed).
If the dependencies are not found, a download may be automatically started, but instead of a specific version, the latest available code of the master/main branch is pulled via git (see the cmake/*.cmake files), these however provide the same compatibility issues.

Therefore, the only properly working setup would be one where the correct versions (those stated in the nix configuration) are manually downloaded, and the paths would then be set via the corresponding environment variables for cmake to find them.

For this reason I have started work on updating some of the cmake configuration in: https://github.com/Nathan-Mossaad/arancini-exploration/tree/cmake-improvements there the correct versions are downloaded and xed configured to result in a properly working txlat build on fedora42 / aarch64.

Additionally, there is the issue of the linker script (aarch64.exec.lds) being currently incompatible with most modern linkers. I have tested it extensively with versions of mold, lld and ld bundled with fedora41, fedora42 and ubuntu 24.04 on different Ampere® Altra® Q80-30 VMs.
Currently, my workaround is that of creating a small environment with the following flake.nix:

{
  inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";

  outputs = { nixpkgs, ... }: {
    devShell.aarch64-linux = let
      pkgs = import nixpkgs { system = "aarch64-linux"; };
    in pkgs.mkShell {
      packages = [
      ];
    };
  };
}

and passing in --cxx-compiler-path "nix develop /path/to/nix.flake-folder --command 'g++'" to txlat

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess for those other linkers you need to regenerate the scripts as described in docs/linker_script.md
But fair enough, "as is" only nix works.

Copy link
Contributor

@ReimersS ReimersS left a comment

Choose a reason for hiding this comment

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

LGTM
Haven't had the patience to check if anything besides formatting changed.

@ReimersS
Copy link
Contributor

Just change all the comments as well so the format_check succeeds.

@Nathan-Mossaad
Copy link
Contributor Author

Nathan-Mossaad commented May 23, 2025

Sorry about the missing formatting, it should now be fixed.

@ReimersS ReimersS merged commit f2028f6 into binary-translation:main May 25, 2025
1 of 5 checks passed
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