bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Matheus Afonso Martins Moreira
Subject: Re: Re: Re: Re: [PATCH 0/4] Add import builtin
Date: Wed, 08 May 2024 11:09:15 +0000

> I've checked the behavior, where the `source' builtin seems to still
> load the non-executable one found first in the path.

I was wrong about that. I derived find_user_library
from find_user_command. That's what I had in mind.

> How do you define `the libraries' out of a wider category of Bash
> scripts intended to be sourced?

Intended purpose. As I've described in other emails,
the lines can get blurry. The reliable way is to look at
the way the developer intended it to be used.

> No. I just think it's unreasonable to force everyone to use a specific
> way of module management that you are trying to invent without
> referencing existing implementations.

No one's being forced to do anything though.

> Right, but the reserve is not true either. I don't think everything
> that is significantly complicated to show the power of Bash should be
> a builtin.

I never used that argument to begin with.
I always maintained that building this into
bash is the better solution.

In these discussions we've managed to
discover at least two *concrete* reasons
why it should actually be a builtin.

 - It can avoid touching PATH.
   Pure shell import cannot.

 - It's compatible with restricted shells.
   Most module managers resolve the path
   and pass in absolute file names, as you noted.

These points are not "personal hangups".
Those are real limitations, present in module managers,
that this patch overcomes by virtue of being a builtin.

> The number of lines being 203 is definitely large.

Definitely large? I don't know about that.
I even split it up into logical changes.
I have no idea what the problem is.
Are you saying it's difficult to review the patch?

> I now know the reason. There are additional refactorings.

So you hadn't actually read the patch until now?

> Right. At the same time, more or less, it is the user's responsibility
> to avoid name conflicts between a script file and an executable file
> just like it is between two script files to be sourced.

Why is responsibility being brought up?
Bash can help the user by supporting separate paths.
If bash can help the user, then it should help the user.
Empowering and helping the user
is the software's reponsibility.

> But I agree that the situation of the mixed script files
> and executable files can be improved.

Then we agree.

> After thinking about it, I think the suggested feature itself should
> probably be disabled under the restricted shell

Not our call to make. It's up to the system's administrator.
They could whitelist some libraries for restricted use.

> similar features such as `enable -f' and `command -p'
> are also disabled in the restricted shell.

The most similar feature at all is the source builtin
and it's not disabled, it's just restricted to file names
that don't contain slashes. Pure bash module managers
are just straight up incompatible with this if they depend
on the ability to pass an absolute file name to source.
The builtin option approach does not have this limitation.

> If it would be enabled, BASH_SOURCE_PATH should be
> at least readonly the same as PATH.

That feature will be in the v2 patch set
which I'm already preparing. 

> I think it's unrealistic to ensure all the existing codes
> that prepare restricted shells to be updated to care
> about BASH_SOURCE_PATH before Bash is updated

Restrictions are enforced after startup files are read.
In case of restricted shell, the libraries path could be
set to the empty string. That'd require the administrator
to explicitly whitelist a set of paths for source, avoiding
any potential problems.

> Of course the topic is different, but the logic is similar.

I'm just gonna accept that. Otherwise this will derail
into a discussion about XDG directories.

> I think they are still serious, but they don't have time to explain
> everything in detail.

The whole point of discussing things is to get those details though.

> Your statement "My only request here is that you seriously consider
> the work of a fellow software developer for merging into master.
> That's all." read like so.

I asked only that you read the patch and evaluate its merits.
At no point did I demand that it be merged.

Evaluating/criticizing the code? That's absolutely fine.
Not even reading the patch though?
That just signals I'm beneath your consideration.

> turned out that there are "refactorings"
> unrelated to the present purpose.

Yes, there are refactorings.
I moved some code around.
I turned hard coded strings into function parameters.

  -- Matheus



reply via email to

[Prev in Thread] Current Thread [Next in Thread]