Skip to content

Remove gnu subdirectory #936

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

keith-packard
Copy link
Collaborator

Get rid of the extra subdirectory for the gnu toolchains and let them sit at the same level as the llvm toolchain. This avoids making the SDK incompatible with previous versions.

Get rid of the extra subdirectory for the gnu toolchains and let them
sit at the same level as the llvm toolchain. This avoids making the SDK
incompatible with previous versions.

Signed-off-by: Keith Packard <keithp@keithp.com>
Copy link
Member

@stephanosio stephanosio left a comment

Choose a reason for hiding this comment

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

Get rid of the extra subdirectory for the gnu toolchains and let them sit at the same level as the llvm toolchain

That makes the top-level SDK directory a bit messy and potentially confusing.

This avoids making the SDK incompatible with previous versions.

Given that previous Zephyr versions will likely not work as-is with the SDK 0.18 because of the GCC version change, does this really make sense?

@keith-packard
Copy link
Collaborator Author

Get rid of the extra subdirectory for the gnu toolchains and let them sit at the same level as the llvm toolchain

That makes the top-level SDK directory a bit messy and potentially confusing.

I disagree; it places all of the toolchains at the same level, with llvm in parallel with the various gcc-derived instances. With this change, the gcc toolchain paths are .../zephyr-sdk-0.18.0-alpha4/gnu/.../bin while the llvm paths are
.../zephyr-sdk-0.18.0-alpha4/llvm/bin which seems inconsistent to me.

This avoids making the SDK incompatible with previous versions.

Given that previous Zephyr versions will likely not work as-is with the SDK 0.18 because of the GCC version change, does this really make sense?

I think so? I have numerous scripts to deal with the SDK in my environment and the change broke them such that I now need separate scripts for the old SDK vs the new one. By removing the additional 'gnu' directory in the middle of the hierarchy, my scripts all work again.

0.18 is going to be a big enough change, let's at least keep as much the same as we can manage?

@keith-packard
Copy link
Collaborator Author

as an anecdote, it took me about 20 minutes to sort out my failing builds due to this additional directory; I'd like to think that we have an opportunity to save some time for a few other developers if we go back to the old structure.

@stephanosio
Copy link
Member

That makes the top-level SDK directory a bit messy and potentially confusing.

I disagree; it places all of the toolchains at the same level, with llvm in parallel with the various gcc-derived instances.

The problem is that, in the current SDK directory structure, the GNU toolchain directory names do not denote that they are GNU toolchains.

For example, the arm-zephyr-eabi may very well be GNU ARM toolchain, LLVM toolchain with only ARM target support enabled, or any other toolchain that targets arm-zephyr-eabi triplet.

With this change, the gcc toolchain paths are .../zephyr-sdk-0.18.0-alpha4/gnu/.../bin while the llvm paths are .../zephyr-sdk-0.18.0-alpha4/llvm/bin which seems inconsistent to me.

Well, the bin directory directly being under the {SDK_ROOT}/llvm is a mere coincidence though; it could very well have been {SDK_ROOT}/llvm/{version}/bin, {SDK_ROOT}/llvm/sysroot/bin or anything else -- it was never intended to be handled in the way you imply.

as an anecdote, it took me about 20 minutes to sort out my failing builds due to this additional directory; I'd like to think that we have an opportunity to save some time for a few other developers if we go back to the old structure.

We can note this change down more clearly in the release notes. After all, SDK 0.18 will contain multiple breaking changes and people will need to read the release notes to understand what they are.

0.18 is going to be a big enough change, let's at least keep as much the same as we can manage?

I would argue that it is better to make all breaking changes at once than to make them incrementally, if we are going to be making them in the future anyway; that way, we can maximise the compatibility range of future SDK releases.

@keith-packard
Copy link
Collaborator Author

I would argue that it is better to make all breaking changes at once than to make them incrementally, if we are going to be making them in the future anyway; that way, we can maximise the compatibility range of future SDK releases.

We never need to move the gnu toolchains into a subdirectory. They can live in the same place as previous SDKs forever. It's purely a matter of taste, and in this case the convenience of not breaking existing tooling seems worth it to me.

I'd suggest the correct clean-up would be to merge all of the GNU toolchains into a single directory, just like LLVM, with
a single gnu/bin and gnu/lib. That's how gnu toolchains are "meant" to be installed, after all. That would also save a bit of space by sharing duplicate docs and scripts.

@stephanosio
Copy link
Member

We never need to move the gnu toolchains into a subdirectory. They can live in the same place as previous SDKs forever. It's purely a matter of taste.

Moving the GNU toolchains into a subdirectory is part of a clean-up of the SDK root directory organisation that was, honestly, a mess prior to it; for instance, we used to have:

SDK_ROOT/
  cmake/           CMake scripts
  sysroots/        Host tools
  *-zephyr-elf/    GNU toolchains

You can see that sysroots can be quite a confusing name for the host tools root directory, and *-zephyr-elf is not so clear about its identity (i.e. GNU toolchain), especially now that we have two different types of toolchains in the SDK (i.e. GNU and LLVM).

After the clean up, we have:

SDK_ROOT/
  cmake/      CMake scripts
  hosttools/  Host tools
  gnu/        GNU toolchains
  llvm/       LLVM toolchains

After all, it is mostly a cosmetic change and thereby subject to personal taste; though, there are some functional aspects to it as well which I will describe below.

I'd suggest the correct clean-up would be to merge all of the GNU toolchains into a single directory, just like LLVM, with
a single gnu/bin and gnu/lib. That's how gnu toolchains are "meant" to be installed, after all. That would also save a bit of space by sharing duplicate docs and scripts.

One of the goals with this clean-up is to make each SDK component more self-contained. While it would be more "UNIX-like" to install all GNU toolchains under the same prefix, doing so would make each GNU toolchain less self-contained and can potentially lead to conflicts among them, especially now that we have "normal GCC" and "ARC GCC" which can be of different versions; installing them under the same prefix would not be so wise.

Also, having each GNU toolchain (or any other SDK component for that matter) self-contained would make it easier to manage them -- for instance, if I would like to uninstall ARC GNU toolchain, I can simply delete the gnu/arc-zephyr-elf directory. Similarly, if I would like to uninstall all GNU toolchains, I can simply delete the gnu directory. That can be easily automated too in case if we ever decide to write some kind of "SDK component manager" script (setup.sh with menuconfig interface?).

in this case the convenience of not breaking existing tooling seems worth it to me.

This is not some major change that would require one to completely rewrite their scripts, and 0.18 is an SDK release that contains quite a few breaking changes already; so, I do not really see how "not breaking existing tooling" is relevant in this case.

@keith-packard
Copy link
Collaborator Author

This is not some major change that would require one to completely rewrite their scripts, and 0.18 is an SDK release that contains quite a few breaking changes already; so, I do not really see how "not breaking existing tooling" is relevant in this case.

With this PR, my old scripts 'just worked'. No changes required. I understand the desire to gather all of the gcc-based toolchains in one directory, but it's a minor cleanup that has no impact on SDK users. Nice, but of limited value. Breaking my scripts was 20 minutes of wasted time, and I'd like to spare other developers from a similar adventure.

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