Re: Re: [PATCH 0/4] Add import builtin

[Koichi Murase]

2024年5月3日(金) 14:45 Matheus Afonso Martins Moreira <matheus@matheusmoreira.com>:
> This is suboptimal. In case the library script cannot found
> in the intended directories, it will needlessly search every
> other entry in PATH for it, despite the fact they will contain
> only unsuitable executables and programs.

There is no doubt that it is more optimal to skip the unnecessary
search, but it doesn't need to be implemented as a new builtin. The
question is whether such a micro-optimization deserves a new builtin.
As far as the scripts are properly set up, the search failure
shouldn't happen. This micro-optimization would only have an effect in
the situation where the shell program does not work. Usually, we do
not care about the performance in a situation where we need to report
an error.

> It also raises the possibility of false positives:
> finding a different executable with the same name
> and sourcing it, causing hard to debug problems.

Yes, that is a problem in directly using the source builtin, but I
think the original intention for the design of `.' and `source' is
that the library shouldn't have the same name as exiting commands.
Again, this is related to the improper setup of the library. Then,
there is always the possibility of a name conflict between different
libraries that share their names even if it doesn't conflict with
executable files. Yeah, but sourcing an executable file can be more
problematic than sourcing a different library.

> >  [...], but you can/should put the files without adding the
> >  executable permission to avoid the confusion you mention.
>
> That's true and a good point. It was my intention to
> reduce the number of candidate directory entries as
> much as possible. Would you say that it would never
> be appropriate for a shell script library to be executable?

As far as it is not intended to be an executable file, yes. By "an
executable file", I mean a file that will be executed in a separate
process.

However, I shall note that some shell programs are intended to be used
as both a library and a separate program. This is also pointed out by
Phi. For example, `modernish' from the modernish framework is
basically intended to be a script to source but can also be used as a
separate program. When it is used as a separate program, for example,
it can be used to retrieve information about the library and setups
and can also be used to run tests. The main file of my framework,
`ble.sh', also behaves in a similar manner, though I do not give the
executable permission to it by default and instead instruct the user
to use `bash /path/to/ble.sh [options]'. Nevertheless, I know that
some users put a symbolic link to `ble.sh' in ~/bin or somwhere and
adds the executable permission to `ble.sh'.

> It might be useful to reverse that condition: make it so
> only non-executable files can be imported instead.
> What do you think?

For the above reason, I think you still need to search for both types of files.

> What if instead of a new builtin, we added options to source?
> This:
>
>     import some-file
>
> Could become this:
>
>     source -i some-file
>     source --import some-file

I think this is more acceptable compared to the new `import' builtin,
but I still think it can be a shell function (or an alias to work
around libraries not using `declare -g' as Chet mentioned).

> >  If this builtin would be included, I think this should be a loadable
> >  builtin that the user can enable as an option.
>
> That's a good idea. When I studied the source code,
> there were comments saying the builtins could be
> turned off and even programatically detected
> via the enable builtin. I made use of those
> facilities so the import builtin should already
> have these properties.

Yes, all the builtins can be enabled/disabled by the `enable' builtin.
But what I wanted to say is that the `import' builtin should be an
opt-in feature, i.e., it should be disabled by default to avoid
conflicts with existing `import'.

> I'm not sure if it's a dynamically loadable module, but it probably could be.

It's not in the current patch but can be modified to be a loadable builtin.

> > it should better be a script file in /examples/functions
>
> In this case, would every script automatically
> have access to the import function?

No, those files need to be distributed by the Bash package of
distributions, and the path needs to be properly set up.

> >  so simple that I doubt it should be defined as a separate library
> >  file. It can be copied directly into the script file.
>
> It could, but that opens the door to variation.

>From your perspective, I understand you are trying to invent the
standard. However, it seems to me that you are trying to add even
another variation (which is less functional than the existing
implementations) to the market.

https://xkcd.com/927/

You are overwriting `import' of the Bash Infinity framework [1]. As
other implementations, Bash Line Editor has `ble-import' [2]. The
Basher framework has `include' [3]. Modernish has `use' [4]. There are
also frameworks developed by the authors of popular shell frameworks.
E.g., the author of `autoenv' provides `basalt.load' [4] in the Basalt
framework. The author of `shellspec' provides `IMPORT'  [5] in the
`modulesh' repository. There are also internal utilities used by
frameworks, such as `__load_completion' and `_comp_load' [7] by
`bash-completion', and `_omb_module_require' [8] by Oh My Bash. I've
never intentionally tried to collect information about different
implementations of the "import", so more frameworks that I haven't
noticed should be providing the functionality of searching and
sourcing the script files.

[1] https://github.com/niieani/bash-oo-framework?tab=readme-ov-file#using-import
[2] 
https://github.com/akinomyoga/ble.sh/wiki/Manual-%C2%A78-Miscellaneous#user-content-fn-ble-import
[3] 
https://github.com/basherpm/basher?tab=readme-ov-file#sourcing-files-from-a-package-into-current-shell
[4] https://github.com/modernish/modernish?tab=readme-ov-file#simple-form
[5] 
https://github.com/bash-bastion/basalt/blob/main/docs/reference/api.md#basaltload
[6] https://github.com/ko1nksm/modulesh/blob/master/README.md#import
[7] 
https://github.com/scop/bash-completion/blob/c48854850f702604aeb600ae496fb6b2eb840f8e/bash_completion#L3127
[8] 
https://github.com/ohmybash/oh-my-bash/blob/9cfb35c946953cb8cc1511cce47986cdd630852a/oh-my-bash.sh#L52

Those implementations of `import' are developed based on the
experience in the real use cases in large-scale shell frameworks. Is
your design of the `import' builtin based on any real frameworks?
Also, if you are going to propose "the standard mechanism", you will
have to survey those implementations to investigate what would be
demanded by the real use cases.

I also think the variation is actually needed because the assumed
search paths are different for different libraries. We cannot simply
merge all those implementations into a single command because each
library wants to limit BASH_IMPORT_PATH to the locations they provide.
Then, it's identical to `PATH=<path> source'. I also think the
variation is not a problem. The separation of the domain is useful.

> Variation in such critical infrastructure
> has led to enormous pain for developers
> working in other languages which have
> multiple semi-compatible or incompatible
> module systems for example. They end up
> having to support several systems.

There are already implementations by multiple module systems as raised
above, though they are not compatible with each other. If desired, the
developer can choose one module framework and write their module for
that module system. Or the developer can develop own module system. You
seem to suggest just the `import' builtin without an actual module
system, but I'm not sure if it's useful to try to kill all existing
module frameworks and start from the beginning. I think you can first
create the module system based on a shell-function implementation of
`import', develop the ecosystem including the standard shell
libraries, and advertise it. Then, if it is widely accepted, it can be
a built-in feature of Bash.

> > Even if it wouldvbe defined in a library script file,
> > you need to anyway specify a path
> > in BASH_IMPORT_PATH to use that function.
>
> With sensible defaults, this problem is mitigated
> if not completely eliminated.

If that works, you can define and export the shell function `import'
within those "sensible defaults" by the user or by the distribution.