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.

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

find boot files relative to executable by default #755

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Conversation

mflatt
Copy link
Contributor

@mflatt mflatt commented Nov 14, 2023

This PR builds on #732 by changing the default boot-file search path to

     "%x:%x/../lib/csv%v/%m:%x/../../boot/%m"

which searches in the executable's directory, through a relative path that works post-installation, and through a relative path that works within the workarea. (More precisely, the middle path in the default takes into account the configured bin and lib target directories.)

After the change, it's much easier to run a just-built executable before installing. Also, installing produces something that you can move around in the filesystem.

As a further step to make things work in the default way, configure is changed to recognize --prefix as an alias for --installprefix. The --installabsolute flag uses the old default search path, which is absolute relative to the target installation directory.

Potential discussion points:

  • I'm not sure about putting %x as the first search path. The intent is to be consistent with Windows and to make it easy for someone to gather files into a single directory and still have scheme run.

  • Instead of changing the default configuration while adding --installabsolute to get the old bevahior, the new behavior could be opt-in with --installabs. I think the new behavior is easier to work with, but the extra path operations and searching aren't useful or unnecessary in a conventional installation.

  • The main program communicates argv[0] to %x resolution through a new Sregister_argv0 function. That function isn't needed on platforms and configurations where the process executable's path can be discovered, but using argv[0] supports a general and portable fallback.

c/self-exe.c Outdated Show resolved Hide resolved
csug/foreign.stex Outdated Show resolved Hide resolved
@ur4t
Copy link
Contributor

ur4t commented Nov 15, 2023

  • I'm not sure about putting %x as the first search path. The intent is to be consistent with Windows and to make it easy for someone to gather files into a single directory and still have scheme run.
  • Instead of changing the default configuration while adding --installabsolute to get the old bevahior, the new behavior could be opt-in with --installabs. I think the new behavior is easier to work with, but the extra path operations and searching aren't useful or unnecessary in a conventional installation.

Consistency across platforms is great. Maybe we can go further to uniform DEFAULT_HEAP_PATH with "%x" SEARCHPATHSEP "%x" PATHSEP ".." PATHSEP "lib" PATHSEP "csv%v" PATHSEP "%m" SEARCHPATHSEP "%x" PATHSEP ".." PATHSEP ".." PATHSEP "boot" PATHSEP "%m" in scheme.c.

  • The main program communicates argv[0] to %x resolution through a new Sregister_argv0 function. That function isn't needed on platforms and configurations where the process executable's path can be discovered, but using argv[0] supports a general and portable fallback.

With Sregister_argv0() and a global process_argv0, argument execpath in find_boot(), next_path() and Sbuild_heap() is redundant, and related logic in main() can be improved as well.

Changing signature of Sbuild_heap() will introduce a API breakage, but it is also not a good idea for two APIs to affect the same global state in different ways.

@mflatt
Copy link
Contributor Author

mflatt commented Nov 15, 2023

Consistency across platforms is great. Maybe we can go further to uniform ...

That path is not quite right in general, though, because the actual middle search path depends on the relative path from InstallBin to InstallLib. Also I don't think it's common arrange files in that way on Windows. So, I doubtful that we should go further in that direction.

it is also not a good idea for two APIs to affect the same global state in different ways.

On reflection, Sregister_argv0() seems like a bad idea, and I was using process_argv0 in unnecessary places. I now think it's better to add Sregister_boot_executable_relative_file() as a variant of Sregister_boot_file to be preferred when the boot file path might be resolved relative to the executable. That way, execpath is consistently passed as an argument when it is useful, including for Sbuild_heap. I rewrote the documentation on the Sregister_ functions, and I renamed and added documentation for a variant that I had added earlier but not documented.

@mflatt mflatt force-pushed the relboot branch 2 times, most recently from 8219e9c to eab4f5c Compare November 16, 2023 00:01
csug/foreign.stex Outdated Show resolved Hide resolved
Building on recent support for "%x" everywhere, the change here makes
the default boot-file search path

 "%x:%x/../lib/csv%v/%m:%x/../../boot/%m"

which searches in the immediate directory, then in the place relative
to the executable's install location to reach installed boot files,
and then relative to the workarea executable to reach workarea boot
files. More precisely, the middle path in the default takes into
account the configured bin and lib target directories.

As a result of this change, it's much easier to run a just-built
executable before installing it, and installing to a non-system
directory produces something that you can move around in the
filesystem.

As a further step to make things work in the default way, `configure`
now supports `--prefix` as an alias for `--installprefix`.

The `--installabsolute` flag uses the old default search path, which
is absolute relative to the target installation directory.
@mflatt mflatt merged commit 43c33ba into cisco:main Nov 16, 2023
13 checks passed
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.

3 participants