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

Jump to bottom

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+94 −32
Open

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

Show file tree
Hide file tree
Changes from 1 commit
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: 8 additions & 3 deletions cmake/fetch_llvm.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,16 @@ if(NOT VERSIONS_JSON)
endif()
read_repo_version(llvmproject llvm-project)

set(llvm_patch_script ${CMAKE_CURRENT_LIST_DIR}/patch_llvm.py)
set(patch_script ${CMAKE_CURRENT_LIST_DIR}/patch_repo.py)
if(GIT_PATCH_METHOD STREQUAL "am")
set(patch_script_args --method am)
elseif(GIT_PATCH_METHOD STREQUAL "apply")
set(patch_script_args --method apply)
endif()
set(patch_dir ${CMAKE_CURRENT_LIST_DIR}/../patches)
set(LLVM_PATCH_COMMAND ${Python3_EXECUTABLE} ${llvm_patch_script} ${patch_dir}/llvm-project)
set(LLVM_PATCH_COMMAND ${Python3_EXECUTABLE} ${patch_script} ${patch_script_args} ${patch_dir}/llvm-project)
if(APPLY_LLVM_PERFORMANCE_PATCHES)
set(LLVM_PATCH_COMMAND ${LLVM_PATCH_COMMAND} && ${Python3_EXECUTABLE} ${llvm_patch_script} ${patch_dir}/llvm-project-perf)
set(LLVM_PATCH_COMMAND ${LLVM_PATCH_COMMAND} && ${Python3_EXECUTABLE} ${patch_script} ${patch_script_args} ${patch_dir}/llvm-project-perf)
endif()

FetchContent_Declare(llvmproject
Expand Down
11 changes: 9 additions & 2 deletions cmake/fetch_newlib.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,21 @@ if(NOT VERSIONS_JSON)
endif()
read_repo_version(newlib newlib)

set(newlib_patch ${CMAKE_CURRENT_LIST_DIR}/../patches/newlib.patch)
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.

There's some code duplication here. Perhaps it could be refactored into (yet) another include file that would define patch_dir, and a combined patch_script_command list variable containing all of ${Python3_EXECUTABLE} ${patch_script} ${patch_script_args}?

Then the LLVM file would still be able to have its special case of conditionally adding a second invocation of the whole thing, but there'd be just one place to add further refinements like GIT_PATCH_METHOD (which I like!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would be neater, so I've wrapped it into a simple function which generates the required command for a given target folder, which the LLVM version can simply call twice.

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 you've slightly improved on my suggestion, by folding all use of patch_dir into the include file instead of having it just defined there and used by the caller. I like it!

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

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
15 changes: 9 additions & 6 deletions cmake/fetch_picolibc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,21 @@ if(NOT VERSIONS_JSON)
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
)
set(patch_script ${CMAKE_CURRENT_LIST_DIR}/patch_repo.py)
if(GIT_PATCH_METHOD STREQUAL "am")
set(patch_script_args --method am)
elseif(GIT_PATCH_METHOD STREQUAL "apply")
set(patch_script_args --method apply)
endif()
set(patch_dir ${CMAKE_CURRENT_LIST_DIR}/../patches)
set(PICOLIBC_PATCH_COMMAND ${Python3_EXECUTABLE} ${patch_script} ${patch_script_args} ${patch_dir}/picolibc)

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
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 <[email protected]>
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