Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the same patching script and command for all repos #584

Merged
merged 3 commits into from
Dec 2, 2024
Merged

Use the same patching script and command for all repos #584

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
591aeb8
Use the same patching script and command for all repos
dcandler Nov 28, 2024
88c5d02
fixup! Use the same patching script and command for all repos
dcandler Nov 29, 2024
6cb16d3
fixup! Use the same patching script and command for all repos
dcandler Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions cmake/fetch_llvm.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,24 @@
# top level to any library builds to prevent repeated checkouts.

include(FetchContent)
include(${CMAKE_CURRENT_LIST_DIR}/patch_repo.cmake)

if(NOT VERSIONS_JSON)
include(${CMAKE_CURRENT_LIST_DIR}/read_versions.cmake)
endif()
read_repo_version(llvmproject llvm-project)

set(llvm_patch_script ${CMAKE_CURRENT_LIST_DIR}/patch_llvm.py)
set(patch_dir ${CMAKE_CURRENT_LIST_DIR}/../patches)
set(LLVM_PATCH_COMMAND ${Python3_EXECUTABLE} ${llvm_patch_script} ${patch_dir}/llvm-project)
get_patch_command(llvm-project llvm_patch_command)
if(APPLY_LLVM_PERFORMANCE_PATCHES)
set(LLVM_PATCH_COMMAND ${LLVM_PATCH_COMMAND} && ${Python3_EXECUTABLE} ${llvm_patch_script} ${patch_dir}/llvm-project-perf)
get_patch_command(llvm-project-perf llvm_perf_patch_command)
set(llvm_patch_command ${llvm_patch_command} && ${llvm_perf_patch_command} )
endif()

FetchContent_Declare(llvmproject
GIT_REPOSITORY https://github.com/llvm/llvm-project.git
GIT_TAG "${llvmproject_TAG}"
GIT_SHALLOW "${llvmproject_SHALLOW}"
GIT_PROGRESS TRUE
PATCH_COMMAND ${LLVM_PATCH_COMMAND}
PATCH_COMMAND ${llvm_patch_command}
# Add the llvm subdirectory later to ensure that
# LLVMEmbeddedToolchainForArm is the first project declared.
# Otherwise CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT
Expand Down
6 changes: 3 additions & 3 deletions cmake/fetch_newlib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@
# top level to any library builss to prevent repeated checkouts.

include(FetchContent)
include(${CMAKE_CURRENT_LIST_DIR}/patch_repo.cmake)

if(NOT VERSIONS_JSON)
include(${CMAKE_CURRENT_LIST_DIR}/read_versions.cmake)
endif()
read_repo_version(newlib newlib)

set(newlib_patch ${CMAKE_CURRENT_LIST_DIR}/../patches/newlib.patch)
get_patch_command(newlib newlib_patch_command)

FetchContent_Declare(newlib
GIT_REPOSITORY https://sourceware.org/git/newlib-cygwin.git
GIT_TAG "${newlib_TAG}"
GIT_SHALLOW "${newlib_SHALLOW}"
GIT_PROGRESS TRUE
PATCH_COMMAND git reset --quiet --hard && git clean --quiet --force -dx && git apply ${newlib_patch}
PATCH_COMMAND ${newlib_patch_command}
# Similarly to picolibc, we don't do the configuration here.
SOURCE_SUBDIR do_not_add_newlib_subdir
)
Expand Down
10 changes: 3 additions & 7 deletions cmake/fetch_picolibc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,20 @@
# top level to any library builss to prevent repeated checkouts.

include(FetchContent)
include(${CMAKE_CURRENT_LIST_DIR}/patch_repo.cmake)

if(NOT VERSIONS_JSON)
include(${CMAKE_CURRENT_LIST_DIR}/read_versions.cmake)
endif()
read_repo_version(picolibc picolibc)

set(
picolibc_patches
${CMAKE_CURRENT_LIST_DIR}/../patches/picolibc/0001-Enable-libcxx-builds.patch
${CMAKE_CURRENT_LIST_DIR}/../patches/picolibc/0002-Add-bootcode-for-AArch64-FVPs.patch
)
get_patch_command(picolibc picolibc_patch_command)

FetchContent_Declare(picolibc
GIT_REPOSITORY https://github.com/picolibc/picolibc.git
GIT_TAG "${picolibc_TAG}"
GIT_SHALLOW "${picolibc_SHALLOW}"
GIT_PROGRESS TRUE
PATCH_COMMAND git reset --quiet --hard && git clean --quiet --force -dx && git apply ${picolibc_patches}
PATCH_COMMAND ${picolibc_patch_command}
# We only want to download the content, not configure it at this
# stage. picolibc will be built in many configurations using
# ExternalProject_Add using the sources that are checked out here.
Expand Down
16 changes: 16 additions & 0 deletions cmake/patch_repo.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@

# Function to generate a PATCH_COMMAND, calling the
# patch_repo.py script using a target set of patches.

function(get_patch_command patch_dir patch_command_out)
set(patch_script ${CMAKE_CURRENT_LIST_DIR}/patch_repo.py)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that using ${CMAKE_CURRENT_LIST_DIR} in this context means it won't be evaluated until the function is called, and then it'll use its caller's value of that variable. So if a function not in the cmake subdir were to call get_patch_command, it would get the wrong directory here.

At the moment this is fine, because all the callers live in that subdir too, but I could imagine it maybe not staying that way. So for one extra safety precaution, how about copying ${CMAKE_CURRENT_LIST_DIR} into a new variable in this include file:

set(patch_list_dir ${CMAKE_CURRENT_LIST_DIR})
function(get_patch_command patch_dir patch_command_out)
    set(patch_script ${patch_list_dir}/patch_repo.py)

and similarly refer to ${patch_list_dir}/../patches?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

True - although setting patch_list_dir inside patch_repo.cmake does not guarantee that the value will still be correct by the time the function is called, since it can be freely modified within the scope. The safest option I can see is pass the toolchain root to the function as an argument, which can then be used within to derive the patch/script locations.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what you mean by that. Yes, something else could modify patch_list_dir, but it would have to do it on purpose – I can't see any plausible way it would happen by accident, in the way that "oops I called get_patch_command from a script somewhere other than the cmake directory" seemed to me like an easy mistake.

But this version works too!

list(APPEND patch_script_args ${Python3_EXECUTABLE} ${patch_script})
if(GIT_PATCH_METHOD STREQUAL "am")
list(APPEND patch_script_args "--method" "am")
elseif(GIT_PATCH_METHOD STREQUAL "apply")
list(APPEND patch_script_args "--method" "apply")
endif()
list(APPEND patch_script_args ${CMAKE_CURRENT_LIST_DIR}/../patches/${patch_dir})

set(${patch_command_out} ${patch_script_args} PARENT_SCOPE)
endfunction()
45 changes: 30 additions & 15 deletions cmake/patch_llvm.py → cmake/patch_repo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env python3

"""
Script to apply a set of patches to llvm-project sources.
Script to apply a set of patches to a git repository.
"""

import argparse
Expand All @@ -18,8 +18,8 @@ def main():
help="Set of patches to apply. This should be a directory containing one or more ordered *.patch files.",
)
parser.add_argument(
"--llvm_dir",
help="Directory of the llvm-project git checkout, if not the current directory.",
"--repo_dir",
help="Directory of the git checkout, if not the current directory.",
)
parser.add_argument(
"--method",
Expand All @@ -36,10 +36,20 @@ def main():
action="store_true",
help="If a patch in a series cannot be applied, restore the original state instead of leaving patches missing. Return code will be 2 instead of 1.",
)
parser.add_argument(
"--3way",
action="store_true",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks as if --3way was previously unconditionally added, and now won't be added by default, because this new boolean argument defaults to off? Is that on purpose? I don't see anything in the commit message mentioning that it was.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The --3way option has some side effects that I'm not sure were considered when it was added as the default; namely that it can seemingly apply a patch, but leave conflict markers in place. That will then cause the compile to fail, instead of flagging that there's an issue in patching. The expectation is for a user to deal with the conflicts, so it's less useful in an automated context. Using --3way shouldn't be necessary from what I've tried (and indeed the newlib patch command never used it), so I removed it from the default but left the option to enable it in the script.

I'll update the commit message when squashing to mention all this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fine – that makes sense. If it's disabled on purpose and you say why, I'm happy.

dest="three_way",
help="If the patch does not apply cleanly, fall back on 3-way merge.",
)
args = parser.parse_args()

if args.llvm_dir:
git_cmd = ["git", "-C", args.llvm_dir]
if args.method == "apply" and args.restore_on_fail and args.three_way:
print("--restore_on_fail is incompatible with --3way using apply")
exit(1)

if args.repo_dir:
git_cmd = ["git", "-C", args.repo_dir]
else:
git_cmd = ["git"]

Expand All @@ -57,7 +67,9 @@ def main():
print("\n".join(p.name for p in patch_list))

if args.method == "am":
merge_args = git_cmd + ["am", "-k", "--ignore-whitespace", "--3way"]
merge_args = git_cmd + ["am", "-k", "--ignore-whitespace"]
if args.three_way:
merge_args.append("--3way")
for patch in patch_list:
merge_args.append(str(patch))
p = subprocess.run(merge_args, capture_output=True, text=True)
Expand All @@ -72,8 +84,8 @@ def main():
# git am doesn't give any specific return codes,
# so check for unresolved working files.
rebase_apply_path = os.path.join(".git", "rebase-apply")
if args.llvm_dir:
rebase_apply_path = os.path.join(args.llvm_dir, rebase_apply_path)
if args.repo_dir:
rebase_apply_path = os.path.join(args.repo_dir, rebase_apply_path)
if os.path.isdir(rebase_apply_path):
print("Aborting git am...")
subprocess.run(git_cmd + ["am", "--abort"], check=True)
Expand All @@ -90,10 +102,11 @@ def main():
apply_check_args = git_cmd + [
"apply",
"--ignore-whitespace",
"--3way",
"--check",
str(current_patch),
]
if args.three_way:
apply_check_args.append("--3way")
apply_check_args.append(str(current_patch))
p_check = subprocess.run(apply_check_args)

if p_check.returncode == 0:
Expand All @@ -102,10 +115,11 @@ def main():
apply_args = git_cmd + [
"apply",
"--ignore-whitespace",
"--3way",
str(current_patch),
]
apply_args = subprocess.run(apply_args, check=True)
if args.three_way:
apply_args.append("--3way")
apply_args.append(str(current_patch))
p = subprocess.run(apply_args, check=True)
applied_patches.append(current_patch)
else:
# Patch won't apply.
Expand All @@ -118,10 +132,11 @@ def main():
reverse_args = git_cmd + [
"apply",
"--ignore-whitespace",
"--3way",
"--reverse",
str(previous_patch),
]
if args.three_way:
reverse_args.append("--3way")
reverse_args.append(str(previous_patch))
p_check = subprocess.run(reverse_args, check=True)
print(
f"Rollback successful, failure occured on {current_patch.name}"
Expand Down
34 changes: 33 additions & 1 deletion patches/newlib.patch → ...hes/newlib/0001-Enable-newlib-build.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,40 @@
From 0e1475e5112b986ef5d9caee23dac554c749c54b Mon Sep 17 00:00:00 2001
From: David Candler <david.candler@arm.com>
Date: Thu, 28 Nov 2024 15:31:35 +0000
Subject: [PATCH] Enable newlib build

---
libgloss/aarch64/syscalls.c | 6 ++
libgloss/arm/cpu-init/rdimon-aem.S | 107 ++++++++++++------------
libgloss/arm/crt0.S | 2 +-
libgloss/arm/linux-crt0.c | 2 +-
libgloss/arm/syscalls.c | 6 +-
libgloss/arm/trap.S | 2 +-
libgloss/libnosys/configure | 2 +-
newlib/libc/include/sys/features.h | 2 +
newlib/libc/machine/aarch64/memchr.S | 6 +-
newlib/libc/machine/aarch64/strchr.S | 6 +-
newlib/libc/machine/aarch64/strchrnul.S | 6 +-
newlib/libc/machine/aarch64/strrchr.S | 10 +--
newlib/libc/stdlib/aligned_alloc.c | 1 +
newlib/libc/sys/arm/crt0.S | 2 +-
newlib/libc/sys/arm/trap.S | 2 +-
newlib/libm/machine/arm/sf_ceil.c | 2 +-
newlib/libm/machine/arm/sf_floor.c | 2 +-
newlib/libm/machine/arm/sf_nearbyint.c | 2 +-
newlib/libm/machine/arm/sf_rint.c | 2 +-
newlib/libm/machine/arm/sf_round.c | 2 +-
newlib/libm/machine/arm/sf_trunc.c | 2 +-
21 files changed, 92 insertions(+), 82 deletions(-)

diff --git a/libgloss/aarch64/syscalls.c b/libgloss/aarch64/syscalls.c
index 7343cc61f..2c4b63c17 100644
--- a/libgloss/aarch64/syscalls.c
+++ b/libgloss/aarch64/syscalls.c
@@ -172,6 +172,12 @@ newslot (void)
return i;
}

+int __aarch64_sme_accessible() {
+ int result = 0;
+ asm volatile ( "mrs %x[result], id_aa64pfr1_el1" : [result]"=r"(result) : : );
Expand Down Expand Up @@ -635,3 +664,6 @@ index 64e4aeb9a..c08fa6fed 100644
#include <math.h>

float
--
2.43.0

Loading