Re: [PATCH 0/4] Add import builtin
> as long as there's no expectation that I'll accept a new feature > just because someone implemented it. There is certainly no expectation of the sort. Thanks for considering the patch. Matheus
Re: [PATCH 0/4] Add import builtin
> Surely these module managers do more than simply implement an > alternate search path? Yes. The core feature that they provide, namely module loading, should be native to bash though. Users who need just that core feature need not pull in an entire module manager just to get it. So for those users the code reduction is quite significant. > For the record I think it would be fine to introduce a new variable > BASH_SOURCE_PATH that changes the behavior of "source" in non-POSIX > mode. I don't think "source" requires a new option to use this. That's true and I think it's also a reasonable way to solve the problem. My original idea was to do exactly that. I changed course while developing it because when I thought about it I concluded it had disadvantages. The behavior of the source builtin already has several special cases. https://www.gnu.org/software/bash/manual/html_node/Bourne-Shell-Builtins.html#index-_002e Simply taking the variable into account would add to that complexity. Adding the option to source though allows simple and unambiguous behavior. I'm not at all opposed to the variable solution however. I can implement it that way if it's deemed to be the preferred way. > I didn't say or mean to imply that it was; I was just comparing it > to another suggested feature about which I have a similar opinion. > The presence or absence of patches has nothing to do with it. (By > the by, at least two implementations of XDG startup files have been > submitted in the past.) I apologize if I've given offense. I think I got a bit defensive after I read another email. > And I appreciate the effort you've put in so far. > Everyone has been seriously considering it. Thanks. Matheus
Re: [PATCH 0/9] Add library mode to source builtin
> I'm thinking of the theoretical case in which a user exports BASH_SOURCE_PATH > then runs existing scripts - not necessarily of their own design > that weren't counting on this feature to ever exist. The option solution completely avoids that possibility. Existing scripts do not pass any options to the source builtin. No unexpected incompatibility should arise if it ignores the new variable unless the option is passed. Matheus
Re: [PATCH 0/9] Add library mode to source builtin
> It does not. It searches for readable files. Look at > findcmd.c:find_path_file(). That's true, I stand corrected. I was thinking of the find_user_command function because it's what I based the find_user_library function on. They are all implemented in terms of the find_user_command_internal. The find_user_command function passes FS_EXEC_PREFERRED|FS_NODIRS while find_path_file just passes FS_READABLE. I suppose I should also have passed FS_READABLE. Matheus
Re: [PATCH 2/9] findcmd: parameterize path variable in functions
> No, it should be implemented using a call to find_in_path() with the > appropriate path string and flags. > > I feel like a feature like BASH_SOURCE_PATH should take less than a > dozen lines of code in source_builtin(). I agree. My version took more because I like splitting things into separate functions. I can reorganize the code to be more dense if necessary. Matheus
Re: [PATCH 9/9] variables: define default BASH_LIBRARIES_PATH
> I don't think `$prefix/lib' would be the appropriate place to put the > script files. /lib directory is traditionally intended for the > architecture-dependent binary libraries loaded by ld. Yeah, you're right about that. This should be changed. > All the other resources independent of the architecture > (and thus can be "shared") > can just be put in /share/bash/scripts or something. Sounds good. Please let me know if you agree to any of the following paths: ~/.local/share/bash:/usr/share/bash ~/.local/share/bash/lib:/usr/share/bash/lib The second option clearly denotes the nature of the contents of the directory and is also more future proof in case more types of shared bash data is ever added in the future. Note that the default value of XDG_DATA_HOME is defined to be ~/.local/share. I'm not exactly invested in getting any XDG stuff into bash but it seems like a reasonable enough default for the user's personal libraries directory. Not sure if there's a better choice here but I'm open to other ideas. > Furthermore, the location overlaps with the loadable builtins, which > are not the script files intended to be sourced. You are mixing the > script file with the loadable builtins while trying to separate the > script files from the executable files. The same issue will arise in > another place. I agree. This will be revised. > This is the same as the argument for the long option. Why only > BASH_LIBRARIES_PATH (among the other similar variables) should honor > ~/.local indirectly implied by XDG_DATA_HOME? There's no particular reason why it should honor any one path or another. My goal is to add a directory inside the user's home directory to the BASH_LIBRARIES_PATH by default so that they have a place to put their functions where they will be found by source when they are used in scripts. I'm not particularly invested in any particular file system taxonomy. > In the current state of Bash, any of the XDG user directories > haven't been hard-coded. That's true. I don't really mind if it stays that way. > as you already know, the introduction of any XDG user directories is > one of the sensitive topics in the bug-bash list, so I believe it > should be discussed separately. Yeah I know how contentious that stuff can be. I'd prefer to avoid the issue altogether so that the patch won't be blocked on those grounds. I'm open to discussing it separately though. Not sure if anything will come of it but sure. > It shouldn't slip into the codebase as a part of another change. Only reason I used it is it was the first thing that came to mind. I suppose I've gotten used to treating ~/.local/ as the "user's root". Not really trying to slip in anything. Matheus
Re: [PATCH 8/9] builtins/source: search libraries in library mode
> I expected that the source builtin without the flag would also > consider BASH_LIBRARIES_PATH if present, which is more similar to > Chet's suggestion in the POSIX mode. More specifically, the source > builtin first searches for BASH_LIBRARIES_PATH if present, or > otherwise falls back to PATH and the local files. When the flag is > specified, the fallbacks are disabled. That was my original idea, yes. I'm open to implementing it that way. I decided not to do that because there are a few special cases already in the way bash handles PATH which can be confusing. This way source will behave just as before unless the option is passed, in which case it will not consider PATH at all, leading to unambiguous behavior at all times.
Re: [PATCH 0/4] Add import builtin
On Mon, May 6, 2024, at 1:27 AM, Matheus Afonso Martins Moreira wrote: >> I fail to see how this could possibly save "thousands and thousands >> of lines of code". > > How many lines of code were needed to implement the module managers? > Probably a lot of lines. I guessed it was in the "thousands of lines" > ballpark. > > However many lines that is, this feature would allow me > to reduce them all to zero by not actually needing a > module manager. And the benefit would be multiplied > by all the users of bash which is probably the most > ubiquitous shell there is. That's a lot of benefit. Surely these module managers do more than simply implement an alternate search path? >> You keep talking about this proposal as if it would spark a huge >> sea change in script maintenance, but I just don't see it, at all. > > A native way to source libraries. Built into bash, available to all users. > With the ergonomics you expect it to have. With good defaults, no setup. > > That's an incredibly huge change. > Especially considering how little code > was required to make it happen. > > [...] > >> The sourced files that can be written with your feature available >> are exactly the same as the ones that can be written right now. > > It's possible but it's not as easy or ergonomic as it could be. At this point it doesn't even feel like we're talking about the same thing. > I think the best solution is to just avoid all that by adding another > variable. > This approach resolves all the above concerns. It also reuses bash's > pathname resolution code and has the added benefit of being native > to bash and available to all users, eliminating the requirement that > they implement this themselves or install an external dependency > just to load libraries. For the record I think it would be fine to introduce a new variable BASH_SOURCE_PATH that changes the behavior of "source" in non-POSIX mode. I don't think "source" requires a new option to use this. >> Much like the periodic requests for XDG-organized startup files > > I think I deserve at least a little more credit than that. > This was not some simple drive-by feature request. I didn't say or mean to imply that it was; I was just comparing it to another suggested feature about which I have a similar opinion. The presence or absence of patches has nothing to do with it. (By the by, at least two implementations of XDG startup files have been submitted in the past.) > But I don't think it's fair at all to just dismiss it as a "request". > The code has been written and as far as I can tell it works > and makes bash better than it used to be. I even made sure > to separate the commits into logical changes so that the > the maintainer would be able to cherry pick the general > refactoring improvements even if the overall feature was > rejected in the end. It was my intention to do at least some > good even if this turned out to be a stupid idea in the end. And I appreciate the effort you've put in so far. > My only request here is that you seriously consider the work > of a fellow software developer for merging into master. That's all. Everyone has been seriously considering it. That doesn't mean they have to agree about it. >> BASH_SOURCE_PATH might be convenient >> but is not groundbreaking and probably >> doesn't merit significant changes to the shell. > > I can't speak for others but I'd say this feature > is going to be quite ground breaking for me. > It's going to significantly and permanently > change the way I use bash. I'm going to write > lots of little libraries. I'll release them as free > software too. My dotfiles repository is AGPLv3. I simply don't see why you can't do all of this right now, as many other people do. >> even convinced it merits a new "source" option. > > Why not? Current commands don't need an option to use their associated path-like variables (BASH_LOADABLES_PATH for "enable", CDPATH for "cd"). -- vq
Re: [PATCH 4/9] bashgetopt: define long option shortener function
> It's simply inconsistent in the present form. Why should only the > source builtin have the long form of the option? No other builtins > have the long form of options, which is inconsistent. Some builtins do support long options, even though it seems to not be documented. In fact, I based the long option shortener function on bash pre-existing code. It is a generalization of the shortener found in the builtins/type.def file. There's a note there that the it's for handling obsolescent option formats such as long options with a single dash instead of two: -type -path -all It also converts the standard GNU style long options: --type --path --all Also, I do realize that there's a big all caps warning in the file saying that "THIS SHOULD REALLY GO AWAY". I get the message, but I'm honestly not sure what that's referring to. The obsolescent options, the long options, the hack to handle them? All of this stuff? Not sure. The builtins seem to be quite consistent in their support for --help though. So there's at least some precedent for long option support. > Suppose you would submit an independent patch to introduce > long option of builtins in Bash. Would you do that only > for the source builtin? No. > If you consider the current form would be complete, you should provide > the reason that the other builtins should not have the long form of > the options, while only the source builtin should have a long option. I don't understand why they don't all have long options to begin with. Is there a reason for it? > If one tries to give the long options to all the existing builtins, > that should be discussed separately. Certainly. > I don't think those changes are allowed to slip in as a bonus > of another change on the source builtin. Well I'm not really trying to "slip in" anything. That implies I'm deceptively trying to get my way. I'm not. I just thought it would be a nice feature to have. If it's not acceptable, I can and will remove the long options. > I'm not sure about what Chet thinks, but I don't think those are > really necessary, though it's a better design in general. I'll wait for his review. > I also have thoughts on the way how this was implemented, but I > wouldn't discuss it for now. Please do. If the implementation can be improved, I will do it. I'm already preparing the v2 patchset based on the feedback I received. Matheus
Re: Re: [PATCH 0/4] Add import builtin
> a language that has no support for namespaces, awkward scoping rules, > a problematic implementation of name references, and so on. I could at least attempt to address those issues. Namespacing could be as simple as prepending the module name to the symbols that are being created. Bash seems to have no issues with symbol names such as path.add. It would be crude but effective. A true implementation would consist of completely separate evaluation environments with symbols linked in-between them. However, I imagine that this will not be possible because it would fundamentally change how bash works so thoroughly that it would just become another language. Please elaborate on the scoping and name reference issues. I'll address them if possible. I'd like to propose two additional semantics for source --library: - Load once The library `x` is loaded and evaluated only once in each context, no matter how many times `source -l x` is executed. There could be an additional `-f|--force` flag to override this. - Automatic symbolic namespacing The library name and a dot are automatically prepended to all symbols defined when a file is sourced via `source -l`. So if `x` contains `location=/etc/config`, then `source -l x` introduces the `x.location` variable into the current environment. Valid library file names would be restricted to the same set of characters that bash allows in variable names, excluding the dot. Otherwise the prefix cannot be derived automatically. There could be additional flags to override this: an `-n NAME` option which specifies a custom prefix and an `-N` option which disables namespacing. > provided that the behaviour of the posix mode > were to be left unchanged in all respects. I'll refactor it so that in POSIX mode the builtin behaves exactly as before. Matheus
Re: Re: [PATCH 1/9] builtins: extract file content executor function
> The patches don't apply to the devel branch. You've made patches on > top of the master branch, but the master branch of Bash is just a > release branch, where each commit corresponds to a release. You should > normally work based on the devel branch. I see. I will rebase the patch on top of the devel branch. > This patch is independent of the present change for the source option. > This is unrelated. > > Also, you've factored out one function from `source_builtin(...)' and > moved another function and it to common.c. However, the new function > `execute_file_contents' is still called from only one place of the > original location. That's true. This change is from the original import builtin patch. The import builtin made use of it, but source does not need it. I kept it because the code was more organized this way, at least in my opinion. Perhaps other parts of the code would like to access the file evaluation functionality in the future. In that case, the needed refactoring would already be done. > By exposing this function in `common.h', > this effectively becomes a part of the public > interface for loadable builtins. I did not realize that. Definitely didn't mean to add some new API. I thought it was an internal file for defining shared functions. That's indeed a serious issue and a very good reason to move this function back into builtins/source.def. I can also simply drop this commit and rebase the rest of the patches if the refactoring is considered more trouble than its worth. Are all external functions defined in all files inside the builtins directory exposed as part of a public API/ABI? If that's the case, then it should also be made static as you suggested. Can you break down for me which parts of the code base are public and which are private? This will allow me to avoid making such mistakes in the future. I'd also like to suggest creating a private commons.c file where potentially reusable functionality can be moved to without exporting them. > There is a possibility that we want to later add other parameters > after seeing the actual uses. However, once it becomes a part of > the public interface, we wouldn't be able to modify it later easily. Absolutely agree. Once this becomes part of a public interface, it can't be changed without breaking everything. It's definitely something that will add to the maintenance efforts which is not what I intended to do. > At least the new function should still be defined in `builtins/source.def' > with the static keyword Agree. I will move it back there in the v2 patch set. > but even that way, the change is completely unrelated to the addition > of the new flag to the source builtin. That's true. I thought the refactoring added some value even though it's a remnant of a previous patch set. If it's more trouble than its worth, I can drop it. > To what you are signing off... By putting this line, you imply that > you agree on the rules that the project defines for copyright, etc., > but Bash doesn't specify it as far as I know. > Then, you've signed off what you haven't even checked the existence. I have read the "helping GNU" pages and know that the GNU/FSF projects require copyright reassignment to the FSF. I am willing to do that. I don't really know the process though. I'm also aware that the sign off is not sufficient in this case. I just figured adding that line couldn't possibly hurt. > Once the patches are accepted with a non-trivial amount of codes, > Chet will explain it to you. I see. Then I'll wait for instructions on that matter. Thanks for the feedback, Matheus
Re: [PATCH 0/9] Add library mode to source builtin
On 5/7/24 3:40 PM, Kerin Millar wrote: On Tue, 7 May 2024, at 7:14 PM, Chet Ramey wrote: On 5/7/24 1:42 PM, Kerin Millar wrote: On Tue, 7 May 2024, at 3:27 PM, Chet Ramey wrote: On 5/5/24 3:39 PM, Kerin Millar wrote: Such is the extent to which I concur that I find even -l to be irritating. The option character isn't important. Is it useful to have an additional If it were of no importance at all, once might then choose the character entirely at random. That's not usually what happens. The issue is whether one is needed at all, not whether or not one character irritates you. If you meant something to the effect of "in the case that I decide that having an option is worthwhile, the choice of character is hardly the foremost concern in all of this", then by all means. Pretty much "Don't get hung up on the specific character until we decide we need one. Certainly don't let one irritate you." -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH] tests/cond-regexp3.sub: avoid ERE UB
On 5/7/24 11:55 AM, Grisha Levit wrote: This test fails on systems with recent-ish BSD regex libs, where a backslash followed by an alphabetic chatacter now throws REG_EESCAPE. Thanks for the report. See https://reviews.freebsd.org/D10510 It's an interesting rationale, at least. I suppose more GNU compatibility is better. Chet -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/ OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH 0/9] Add library mode to source builtin
On Tue, 7 May 2024, at 7:14 PM, Chet Ramey wrote: > On 5/7/24 1:42 PM, Kerin Millar wrote: >> On Tue, 7 May 2024, at 3:27 PM, Chet Ramey wrote: >>> On 5/5/24 3:39 PM, Kerin Millar wrote: >>> Such is the extent to which I concur that I find even -l to be irritating. >>> >>> The option character isn't important. Is it useful to have an additional >> >> If it were of no importance at all, once might then choose the character >> entirely at random. That's not usually what happens. > > The issue is whether one is needed at all, not whether or not one character > irritates you. In the course of quoting me, you stated that the character isn't important then proceeded to pose an open question as to whether having an option is worthwhile, so I engaged on both counts. I don't require for anyone's agreement to think that naming matters. I don't believe that anyone that writes software genuinely believes it doesn't. I didn't say what the character ought to be. I simply didn't like the one that was presented and still don't. If you meant something to the effect of "in the case that I decide that having an option is worthwhile, the choice of character is hardly the foremost concern in all of this", then by all means. >>> option to `source' that forces it to use $BASH_SOURCE_PATH, or should that >>> behave like other builtins ($CDPATH, $BASH_LOADABLES_PATH)? >> >> If BASH_SOURCE_PATH is to be able to take its value from the environment, it >> might be useful. > > That's the standard behavior, with only a few exceptions. > > >> That is, to have a means of opting in may be sensible from a backward- >> compatibility standpoint, whether it be in the form of an option character, >> a >> shell option or something else. Otherwise, probably not. I'm thinking of the >> theoretical case in which a user exports BASH_SOURCE_PATH then runs existing >> scripts - not necessarily of their own design - that weren't counting on >> this >> feature to ever exist. > > You can export CDPATH and get the same effect with `cd', or export PATH and > modify command search order. Yes (albeit standard behaviours). I find myself mildly inclined towards the position of not adding more options unless it is considered truly necessary. I have nothing further to add, so will take my leave of this thread. -- Kerin Millar
Re: [PATCH 0/9] Add library mode to source builtin
On 5/7/24 1:42 PM, Kerin Millar wrote: On Tue, 7 May 2024, at 3:27 PM, Chet Ramey wrote: On 5/5/24 3:39 PM, Kerin Millar wrote: Such is the extent to which I concur that I find even -l to be irritating. The option character isn't important. Is it useful to have an additional If it were of no importance at all, once might then choose the character entirely at random. That's not usually what happens. The issue is whether one is needed at all, not whether or not one character irritates you. option to `source' that forces it to use $BASH_SOURCE_PATH, or should that behave like other builtins ($CDPATH, $BASH_LOADABLES_PATH)? If BASH_SOURCE_PATH is to be able to take its value from the environment, it might be useful. That's the standard behavior, with only a few exceptions. That is, to have a means of opting in may be sensible from a backward- compatibility standpoint, whether it be in the form of an option character, a shell option or something else. Otherwise, probably not. I'm thinking of the theoretical case in which a user exports BASH_SOURCE_PATH then runs existing scripts - not necessarily of their own design - that weren't counting on this feature to ever exist. You can export CDPATH and get the same effect with `cd', or export PATH and modify command search order. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/9] Add library mode to source builtin
On Tue, 7 May 2024, at 3:27 PM, Chet Ramey wrote: > On 5/5/24 3:39 PM, Kerin Millar wrote: > >> Such is the extent to which I concur that I find even -l to be irritating. > > The option character isn't important. Is it useful to have an additional If it were of no importance at all, once might then choose the character entirely at random. That's not usually what happens. > option to `source' that forces it to use $BASH_SOURCE_PATH, or should that > behave like other builtins ($CDPATH, $BASH_LOADABLES_PATH)? If BASH_SOURCE_PATH is to be able to take its value from the environment, it might be useful. That is, to have a means of opting in may be sensible from a backward-compatibility standpoint, whether it be in the form of an option character, a shell option or something else. Otherwise, probably not. I'm thinking of the theoretical case in which a user exports BASH_SOURCE_PATH then runs existing scripts - not necessarily of their own design - that weren't counting on this feature to ever exist. On the other hand, bash already has a cornucopia of options and I'm uncertain as to how realistic this concern is. There are various ways for the environment to influence the behaviour of a bash script as it stands. I would be interested to know what others think. -- Kerin Millar
[PATCH] tests/cond-regexp3.sub: avoid ERE UB
This test fails on systems with recent-ish BSD regex libs, where a
backslash followed by an alphabetic chatacter now throws REG_EESCAPE.
See https://reviews.freebsd.org/D10510
---
tests/cond-regexp3.sub | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/cond-regexp3.sub b/tests/cond-regexp3.sub
index d939548c..bef44f8b 100644
--- a/tests/cond-regexp3.sub
+++ b/tests/cond-regexp3.sub
@@ -58,7 +58,7 @@ done
[[ x =~ \\x ]] ; echo $?
bs='\'
-[[ x =~ ${bs}x ]] ; echo $?
+[[ \[ =~ ${bs}[ ]] ; echo $?
[[ x =~ $'\\'x ]] ; echo $?
[[ x =~ '\'x ]] ; echo $?
--
2.45.0
Re: Bash Reference Manual: broken link
On 4/25/24 2:37 AM, Jim Blackson wrote: Hello GNU, 1. Perhaps there is a broken link in the bread crumb list of the index page of the Bash Reference Manual, found at: https://www.gnu.org/software/bash/manual/html_node/index.html Thanks for the report. I'm not sure if this is an artifact of how I created the html files or whether the directory organization on gnu.org has changed since I released bash-5.2. I'll make sure to update this for bash-5.3. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: Possible Typo for "Set" Section of Bash Reference Manual
On 5/6/24 8:32 PM, John wrote: Hi! I believe the Bash Reference Manual is missing a key note for using "set -o". On the man page for "bash" (https://tiswww.case.edu/php/chet/bash/bash.html), the following line is present * If *-o* is supplied with no /option-name/, the values of the current options are printed. If *+o* is supplied with no /option-name/, a series of *set* commands to recreate the current option settings is displayed on the standard output. On the corresponding section for "Set" in the Bash Reference Manual (https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html), that line of text is *not present*. Thanks for the report. The devel branch versions of the man page and the info file have identical text describing this mode. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/ OpenPGP_signature.asc Description: OpenPGP digital signature
Re: [PATCH 2/9] findcmd: parameterize path variable in functions
On 5/7/24 6:35 AM, Koichi Murase wrote: For consistency, shouldn't the suggested feature to change the search path for the script files be implemented by a similar flag CMDSRCH_XXX? No, it should be implemented using a call to find_in_path() with the appropriate path string and flags. I feel like a feature like BASH_SOURCE_PATH should take less than a dozen lines of code in source_builtin(). -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/9] Add library mode to source builtin
On 5/5/24 11:55 PM, Matheus Afonso Martins Moreira wrote: Such is the extent to which I concur that I find even -l to be irritating. Well, I don't concur at all. I think it's just the opposite. I think it's actually the source builtin which incorrectly implies that all sourced scripts are executables when they might actually be libraries and modules. It implies this because the source code for source prefers files with the executable bit set It does not. It searches for readable files. Look at findcmd.c:find_path_file(). -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/9] Add library mode to source builtin
On 5/5/24 3:39 PM, Kerin Millar wrote: Such is the extent to which I concur that I find even -l to be irritating. The option character isn't important. Is it useful to have an additional option to `source' that forces it to use $BASH_SOURCE_PATH, or should that behave like other builtins ($CDPATH, $BASH_LOADABLES_PATH)? -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/4] Add import builtin
On 5/6/24 1:27 AM, Matheus Afonso Martins Moreira wrote: Either you add the library directories to the PATH and risk false positives, or you delete the other directories and make sourced scripts unable to run commands. External commands, but there is nothing preventing something like a `registration' shell function, for example, that the sourced file invokes that runs with a local $PATH. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/4] Add import builtin
On 5/6/24 1:21 AM, Phi Debian wrote: On Sun, May 5, 2024 at 9:47 PM Greg Wooledge wrote: On Sun, May 05, 2024 at 03:32:04PM -0400, Lawrence Velázquez wrote: The idea to add a BASH_SOURCE_PATH variable that gets searched before, or instead of, PATH when using the source builtin -- that sounds good. I don't really understand the concepts behind the rest of the discussion. Well even this is unclear 'BASH_SOURCE_PATH get searched before PATH' or 'BASH_SOURCE_PATH get searched instead of' or even 'BASH_SOURCE_PATH get searched after PATH' Because we haven't discussed the semantics. There shouldn't be any changes in posix mode, so that will continue to use PATH. In default mode, as long as the `sourcepath' option is enabled, if BASH_SOURCE PATH is set it would supersede PATH. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/4] Add import builtin
On 5/7/24 2:43 AM, Kerin Millar wrote: Of course, it is sensible to gauge whether there is any potential interest in a proposed feature prior to submitting patches. Matheus tried to do exactly that on the 21st April. I may think that the resulting patches have been oversold but I would not fault his process. I don't have any problem with his process, as long as there's no expectation that I'll accept a new feature just because someone implemented it. Implementation is once, maintenance is forever. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/4] Add import builtin
On 5/7/24 12:24 AM, Phi Debian wrote: Just questioning, I really have no idea what is the bash dev team way of receiving enhancement request. This list is the traditional place where proposals and discussion happen. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: [PATCH 0/4] Add import builtin
On 5/6/24 9:12 AM, Koichi Murase wrote: Chet mentioned the additional variable for the locations to search, but not a new builtin or a flag. Correct. The $PATH overloading has always been a source of confusion, or at least something you need to get used to; it would be a benefit to have a way to sidestep it without temporarily changing $PATH. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/
Re: Re: Re: [PATCH 0/4] Add import builtin
On Sun, 5 May 2024 at 11:50, Koichi Murase wrote:
> > Ideally, they'll be using bash's native import under the hood!
>
> Yes, module managers still need to implement their own "import"
> command while using the proposed "import" primitive under the hood,
> and it's simply interchangeable with the source builtin we already
> have.
>
> * Module managers typically try to identify the file under more detailed
> rules [...] For these reasons, most module managers actually resolve the
> path by itself using their rules and specify *the absolute path* to the
> source builtin. As far as the absolute path is specified, there is no
> difference between the source builtin and the suggested `source -i' or
> `import'.
>
I wonder if it would be useful to add options to 'command':
* '-o' would report only the first command found (when more than one is
given) (or could be '-1');
* '-p' would skip builtins & functions, and fail silently if no file can
be found;
* '-x' would search for files that *lack* exec permission.
Then a module loader could simply be:
require() {
[[ ${__loaded_from[$1]} ]] && return
local rp
rp=$(
PATH=$LIB_PATH \
command -opvx "$1".bash "$1".sh "$1"
) &&
source "$rp" &&
__loaded_from[$1]=$rp
}
(Being a Perl monk, I like the distinction between 'require' that has
effect during run time, and 'use' that has effect during parsing. And
'import' is the process for binding names into namespaces, not the process
for loading files.)
-Martin
Re: [PATCH 9/9] variables: define default BASH_LIBRARIES_PATH
2024年5月5日(日) 18:57 Matheus Afonso Martins Moreira : > +/* The default value of the BASH_LIBRARIES_PATH variable > + which is used by source -l instead of PATH. */ > +#ifndef DEFAULT_LIBRARIES_PATH > +#define DEFAULT_LIBRARIES_PATH \ > + "~/.local/lib/bash:/usr/local/lib/bash:/usr/lib/bash" I don't think `$prefix/lib' would be the appropriate place to put the script files. /lib directory is traditionally intended for the architecture-dependent binary libraries loaded by ld. All the other resources independent of the architecture (and thus can be "shared") can just be put in /share/bash/scripts or something. Furthermore, the location overlaps with the loadable builtins, which are not the script files intended to be sourced. You are mixing the script file with the loadable builtins while trying to separate the script files from the executable files. The same issue will arise in another place. https://lists.gnu.org/archive/html/bug-bash/2024-05/msg00090.html 2024年5月7日(火) 2:11 Matheus Afonso Martins Moreira : > > an incomplete support for the XDG base directories, which should > > actually be discussed separately. > > XDG does not specify the ~/.local and ~/.local/lib directories. > Only the XDG Data Directory defaults to ~/.local/share if unset. > That implies a ~/.local directory tree that mirrors that of /usr. This is the same as the argument for the long option. Why only BASH_LIBRARIES_PATH (among the other similar variables) should honor ~/.local indirectly implied by XDG_DATA_HOME? In the current state of Bash, any of the XDG user directories haven't been hard-coded. Also, as you already know, the introduction of any XDG user directories is one of the sensitive topics in the bug-bash list, so I believe it should be discussed separately. It shouldn't slip into the codebase as a part of another change.
Re: [PATCH 8/9] builtins/source: search libraries in library mode
2024年5月5日(日) 18:57 Matheus Afonso Martins Moreira : > - filename = search_for_file (list); > + if (!library_mode) > +filename = search_for_file (list); > + else > +filename = search_for_library (list); I expected that the source builtin without the flag would also consider BASH_LIBRARIES_PATH if present, which is more similar to Chet's suggestion in the POSIX mode [1]. More specifically, the source builtin first searches for BASH_LIBRARIES_PATH if present, or otherwise falls back to PATH and the local files. When the flag is specified, the fallbacks are disabled. [1] https://lists.gnu.org/archive/html/help-bash/2024-04/msg00022.html
Re: [PATCH 4/9] bashgetopt: define long option shortener function
2024年5月5日(日) 18:57 Matheus Afonso Martins Moreira : > Define the shorten_long_options helper function that converts Let's talk about whether we should support the long form of the option in this patch set. 2024年5月7日(火) 2:11 Matheus Afonso Martins Moreira : > > incomplete addition of the long option of builtins > > What do you mean? What is missing? It's simply inconsistent in the present form. Why should only the source builtin have the long form of the option? No other builtins have the long form of options, which is inconsistent. Suppose you would submit an independent patch to introduce the long option of builtins in Bash. Would you do that only for the source builtin? If you consider the current form would be complete, you should provide the reason that the other builtins should not have the long form of the options, while only the source builtin should have a long option. If one tries to give the long options to all the existing builtins, that should be discussed separately. I don't think those changes are allowed to slip in as a bonus of another change on the source builtin. I'm not sure about what Chet thinks, but I don't think those are really necessary, though it's a better design in general. I also have thoughts on the way how this was implemented, but I wouldn't discuss it for now.
Re: [PATCH 2/9] findcmd: parameterize path variable in functions
2024年5月5日(日) 18:56 Matheus Afonso Martins Moreira : > The PATH variable is hard coded in the user command finder function. > Transforming it into an argument allows reusing the file finding logic > with variables other than PATH, making the code more flexible. There is already a very similar feature of the `command' builtin, which enforces PATH to be a specific path in searching the command yet doesn't replace the actual value of PATH for the executed command. This is implemented by the flag `#define CMDSRCH_STDPATH 0x02' (findcmd.h:28 @ master). For consistency, shouldn't the suggested feature to change the search path for the script files be implemented by a similar flag CMDSRCH_XXX? This is sufficient because the suggested new parameter `path_var' only becomes PATH or BASH_LIBRARIES_PATH for now. I wouldn't expect a YAGNI argument about the future possibility.
Re: [PATCH 1/9] builtins: extract file content executor function
The patches don't apply to the devel branch. You've made patches on top of the master branch, but the master branch of Bash is just a release branch, where each commit corresponds to a release. You should normally work based on the devel branch. 2024年5月5日(日) 18:56 Matheus Afonso Martins Moreira : > Extract into a dedicated helper function the code which loads > [...] This patch is independent of the present change for the source option. This is unrelated. Also, you've factored out one function from `source_builtin(...)' and moved another function and it to common.c. However, the new function `execute_file_contents' is still called from only one place of the original location. I think you will not be satisfied by just this comment, so I'd explain it in more detail. By exposing this function in `common.h', this effectively becomes a part of the public interface for loadable builtins. However, this abstraction is not motivated by the actual use cases. Without any actual uses, we cannot make sure this specific way of abstraction (such as the set of parameters that the function should receive) would be fine. There is a possibility that we want to later add other parameters after seeing the actual uses. However, once it becomes a part of the public interface, we wouldn't be able to modify it later easily. Otherwise, a loadable builtin makes the process crash. Then, you'll have to prepare `execute_file_contents_v2(...)' or something when the actual use cases reveal a necessity of new parameters or a change of the interface. I think this is a premature abstraction and should be avoided. At least the new function should still be defined in `builtins/source.def' with the static keyword, but even that way, the change is completely unrelated to the addition of the new flag to the source builtin. You might try to claim the usefulness or the validity of this change, but I don't have any objection to its usefulness. It's not the problem of usefulness. > Signed-off-by: Matheus Afonso Martins Moreira To what you are signing off... By putting this line, you imply that you agree on the rules that the project defines for copyright, etc., but Bash doesn't specify it as far as I know. Then, you've signed off what you haven't even checked the existence. Instead, FSF requires the copyright assignment to FSF (which is more strict than just e.g. DCO or anything), which shouldn't be exempted just by "Signed-off-by". Once the patches are accepted with a non-trivial amount of codes, Chet will explain it to you.
Re: Re: Re: [PATCH 0/9] Add library mode to source builtin
Honestly, I share the same feeling as the others about the name `library'. The use of the ``library'' feels something wrong in this context. I still think the variable name should be BASH_SOURCE_PATH. I've suggested `-l' as an *example*, but that's just because I couldn't come up with another letter. It could probably be `-p' following the similar option of `command -p'. The reasons that using "library" in this context feels strange are as follows: * The suggested feature just changes how the script file would be searched. This doesn't define what would be the library in Bash and what would not. The suggested feature can be used to load a library, but the reverse is not true; a library doesn't need to be sourced through the suggested feature. Even if a script file that contains a set of utilities is sourced with an absolute/relative path with a slash, I'd still call it a library. Even if it is found in PATH, I'd still call it a library. Even if it is found in the current working directory, I'd still call it a library. The suggested command-line flag forces the search to be in BASH_LIBRARIES_PATH and disables the search of libraries in PATH and the current working directory. You seem to try to justify the naming by explaining how the script files loaded by the suggested feature can be considered libraries, but to justify the naming you would need to explain also how the script files with utilities located in PATH or the current working directory are NOT considered libraries. If you would try that, I will contest against it. * We haven't been using the term `library' in the Bash codebase to mean the general scripts that contain a set of functions and are intended to be sourced. These have been just called script files without a specific name. When we introduce a new term, I'd expect a clear difference from the existing one. If the new mechanism is incompatible with the conventional script files, it would suffice as a difference. Or if the suggested feature would introduce new mechanisms dedicated to libraries (such as namespacing, dependency resolution, etc.) and those mechanisms are not going to be provided/available with the conventional `source' builtin, it would also suffice as a difference. As far as the new term means essentially a similar thing as existing script files, I don't see a reason to introduce a new term. * I feel not every script file that is intended to be sourced and usefully placed in BASH_LIBRARIES_PATH would be actually a library. It might define aliases and shell functions that the user can interactively call from the command line. It can also be an initialization code for the environment. It can also be just personal configurations. For example, would you call a single line file « eval "$(starship init bash)" » a library? Maybe you call it a library, but I wouldn't.
Re: [PATCH 0/4] Add import builtin
Ok thanx for clarification.
I agry about git{lab,hub} position that's why I added {,whatever} meaning
something really free.
Ok now all is clear and I will not be surprised anymore if some other
'feature' proposal shows up in the future.
