Conversation
https://github.com/jrelvas-ipc/hwcaps-loader The downside of glibc-hwcaps is it can only support loading higher micro-arch _libraries_, some packages may only provide binaries or their binary front-end does not link against their own library The hwcaps-loader mechanism rectifies that
joebonrichie
force-pushed
the
hwcaps-bin-loader
branch
from
d97ce09 to
c9f701f
Compare
EbonJaeger
left a comment
EbonJaeger
left a comment
There was a problem hiding this comment.
I think the post_execute_step() function could benefit from some comments explaining why it's doing what it's doing. For instance, why are we making symlinks in one case, but not the other?
| except FileNotFoundError as e: | ||
| print(f"File not found: {file_path} {e}") | ||
| return False | ||
| except Exception as e: | ||
| print(f"{e}") | ||
| return False |
There was a problem hiding this comment.
Should this use the pretty-printing console_ui functions to print? If they were warnings, it would make it easier to see during the build.
| end_time = timer() | ||
| console_ui.emit_success("Build", "{} successful ({})". | ||
| format(step, timedelta(seconds=end_time-start_time))) | ||
| post_execute_step(context, r_step, step, work_dir) |
There was a problem hiding this comment.
It looks like this is run on every step of the build. I think it would make more sense to check if we're on the "install" step before running it here instead of inside the post_execute_step() function; it also slightly simplifies that function.
| hwcaps_v1_bin_dir = os.path.join(context.get_install_dir(), "usr", "hwcaps", "x86-64-v1", "bin") | ||
| bin_dir = os.path.join(context.get_install_dir(), "usr", "bin") | ||
|
|
||
| if context.avx2 and step_n == "install": |
There was a problem hiding this comment.
Paired with my other suggestion about moving the step check, could we inverse this check and early return if the context is not avx2?
| if is_elf_file(file_path): | ||
| shutil.move(file_path, hwcaps_v3_bin_dir) | ||
|
|
||
| if context.emul32 is False and context.avx2 is False and step_n == "install" and was_avx2_context is True: |
There was a problem hiding this comment.
It would be great if we could also inverse all of this for an early return. There are a lot of elements in this check as well that make it a bit hard to read.
2 participants
https://github.com/jrelvas-ipc/hwcaps-loader
The downside of glibc-hwcaps is it can only support loading higher micro-arch libraries, some packages may only provide binaries or their binary front-end does not link against their own library
The hwcaps-loader mechanism rectifies that