Conversation
The rewrite ignored progname, located the executable through /proc/self/exe or a hardcoded GCC: and then stripped a fixed number of components, so it only ever worked for a program in bin/. Bring back the upstream algorithm, which matches the trailing components of progname against the configured bin prefix and works at any depth.
A colon ends an AmigaDOS volume or assign name, so splitting COMPILER_PATH or PATH on it breaks every element. Give the AmigaOS host an xm header with PATH_SEPARATOR ';' and drop the collect2 workaround that hardcoded GCC:bin instead of reading PATH: the driver's environment now reaches collect2 through the local variables libnix's setenv mirrors.
The shell keeps its search path in the process rather than in PATH, so make_relative_prefix cannot find a program given by bare name. AmigaOS records the directory a program was loaded from as PROGDIR:; resolve it to a real path with NameFromLock, since the result is handed to child processes whose own PROGDIR: differs.
The sdk_root spec function computed the SDK root from the libexec prefix and normalized the '/../' it produced by hand. The driver's find-file already searches the startfile prefixes, which include <prefix>/m68k-amigaos/lib/, so the libnix, clib2, ixemul and NDK directories are reachable from there with plain relative paths. Drop sdk_root and the GCC: assign hardcoded for the AmigaOS host: the hosted compiler keeps the cross layout and finds everything relative to the executable. The generic -isystem <prefix>/include and -L <prefix>/lib entries go: those directories hold nothing for the target, and the driver already searches <prefix>/m68k-amigaos/include and lib. Translated to find-file they resolved from the runtime's -B directory instead and put m68k-amigaos/lib in front of clib2/lib, so -mcrt=clib2 linked the wrong libc.a.
codewiz
added
ci-amigaos
There was a problem hiding this comment.
🟡 Changes recommended
The removed prefix helper is still referenced by the AmigaOS C++ driver, and libiberty’s fallback uses the wrong path separator.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Relocates AmigaOS-hosted GCC resources relative to the executable instead of relying on GCC:.
Changes:
- Resolves bare executable names through
PROGDIR:. - Uses semicolon-separated host paths and restores environment-based linker lookup.
- Replaces
sdk_rootspecs with%:find-file.
File summaries
| File | Description |
|---|---|
libiberty/pex-amigaos.c |
Updates environment inheritance documentation. |
libiberty/make-relative-prefix.c |
Restores generic relocation logic with AmigaOS resolution. |
gcc/gcc.cc |
Removes the AmigaOS prefix helper. |
gcc/config/m68k/xm-amigaos.h |
Defines the host path separator. |
gcc/config/m68k/m68kamigaos.h |
Migrates SDK paths to %:find-file. |
gcc/config.host |
Registers the AmigaOS host configuration. |
gcc/collect2.cc |
Restores environment-based linker search paths. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
codewiz
added
ci-amigaos
make-relative-prefix.c picks its own PATH_SEPARATOR, so the PATH walk that runs when PROGDIR: cannot be resolved split AmigaDOS path lists inside the volume names.
TARGET_AMIGAOS comes from amigaos-protos.h through tm_p.h, which the driver never includes, so this block was never compiled. It called the sdk_root helper that no longer exists, and the __init_eh reference it wanted to add is already in LINK_SPEC.
codewiz
added
ci-linux
There was a problem hiding this comment.
🟡 Changes recommended
Executables located at an Amiga volume root lose their volume prefix during relative-prefix calculation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+92
to
+94
| #elif defined (__amiga__) | ||
| /* A colon ends an AmigaDOS volume or assign name. */ | ||
| # define PATH_SEPARATOR ';' |
Member
Author
There was a problem hiding this comment.
Not changing this. An executable directly in a volume root has no prefix to derive: the prefix is the parent of bin/, and a volume root has no parent on AmigaDOS. The installed layout is always /bin/, and that case works: gcc:bin/m68k-amigaos-gcc yields gcc:bin/../lib/... and gcc:bin/../libexec/.... Treating the colon as a directory separator would break it, since the components are rejoined with a slash and SDK:/bin means the parent of the root on AmigaDOS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #38: make the AmigaOS-hosted gcc find its own directories the way every other host does, instead of through a hardcoded
GCC:assign and a spec function that reverse-engineered the prefix.What changes
__amiga__block: when argv[0] is a bare name, resolvePROGDIR:withGetProgramDir/NameFromLockand use that as the program path. The AmigaDOS shell keeps its search path in the process, not inPATH, so the upstreamPATHwalk finds nothing there. The result is a real path because it is handed to child processes whose ownPROGDIR:is a different directory. No argv[0] rewriting, so nothing changes for symlinked programs.PATH_SEPARATOR ';'on the AmigaOS host via a newxm-amigaos.h: a colon ends a volume or assign name, so splittingCOMPILER_PATHorPATHon it breaks every element. collect2 goes back to plainprefix_from_env ("PATH"); the driver's environment reaches it through the local variables libnix'ssetenvnow mirrors (setenv: mirror the environment into local variables libnix#16).%:find-fileinstead ofsdk_root. find-file searches the startfile prefixes, which include<prefix>/m68k-amigaos/lib/, so../libnix/include,../clib2/liband friends resolve from there. Thesdk_rootfunction, its hand-written/../normalizer and theGCC:include dirs are gone; the hosted compiler keeps the cross layout viaCROSS_DIRECTORY_STRUCTUREand finds everything relative to the executable. The generic-isystem <prefix>/includeand-L <prefix>/libentries are dropped: those directories hold nothing for the target, the driver already searchesm68k-amigaos/includeandlib, and translated to find-file they resolved from the runtime's-Bdirectory and putm68k-amigaos/libahead ofclib2/lib, so-mcrt=clib2linked the wronglibc.a.Tested
Linux cross compiler rebuilt and installed from this branch: hello world with
-noixemul,-mcrt=nix13,-mcrt=clib2and a C++ program with libnix all compile, link and run under vamos, and-vshows every include and library directory resolving underm68k-amigaos/.AmigaOS-hosted compiler rebuilt from this branch and run under vamos with the prefix mounted as a plain volume and no
GCC:assign:gcc:bin/m68k-amigaos-gccresolves itself throughPROGDIR:, derives every prefix from it (COLLECT_LTO_WRAPPER=gcc:bin/../libexec/...,-B gcc:bin/../lib/gcc/.../m68k-amigaos/lib/../libnix/lib/), and a C program with libnix, one with clib2 and a C++ program all compile, link through collect2 and run.Known limit for the real AmigaOS host: the resolved paths contain
..components (m68k-amigaos/lib/../libnix/include), which AmigaDOS does not understand, so libnix needs to translate..in paths before this works outside vamos. That is a libnix change, tracked separately.