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

Merge new commits from upstream #1

Open
wants to merge 1,126 commits into
base: dev/osh-release-0.6.pre15
Choose a base branch
from

Conversation

Ruthenic
Copy link

@Ruthenic Ruthenic commented Feb 10, 2022

Seems to work fine? I kept HEAD for all merge conflicts, so I think it should be okay.
Also I wasn't sure which branch to base off of so I just used the one with the most different commits that I saw

scop and others added 30 commits May 22, 2021 23:49
Writing to the source tree may not be allowed.
It is needed now, at least after recent ant and portinstall test in
tmpdir changes.
macOS in GitHub workflows barfs:

    No Java runtime present, requesting install.
    Unable to locate a Java Runtime to invoke.

The exit code of -help is nonzero, so resort to grep to try and detect
this.
Anchor dash detection to beginning of line, so that output like

    printenv: illegal option -- -

...(from macOS) does not cause a match.
So that files can be created in the just created dirs.
temp_cwd is cleaned up automatically.
We might not be allowed to write in the source dir.
Now we can use "@pytest.mark.bashcomp(temp_cwd=True)" for a cleaner
handling of the test-purpose temporary directory.

We now create dummy directories and files in the directory provided by
"temp_cwd".  For this purpose, the code in "prepare_fixture_dir"
(test/t/conftest.py) has been extracted to an independent function
"create_dummy_filedirs".  Tests at "test/t/test_{kdvi,kpdf,evince}.py"
and "prepare_fixture_dir" now call "create_dummy_filedirs".
To properly detect unwanted changes of OLDPWD by completions, we
remove OLDPWD from the white list of mutable variables.  Now, we need
to properly save and restore the value of OLDPWD when we change the
testing directory on purpose.

We add variable names "_bash_completion_test_*" in the white list and
use "_bash_completion_test_OLDPWD" to save the value of "OLDPWD".

Remark: Another suggested solution was to use "pushd" and "popd" in
order to save/restore the directory, but this idea was discarded
because they turned out to affect "OLDPWD" too.
Ant may warn for duplicates:

    Duplicated project name in import. Project P defined first in [...]
    and again in [...]

Even though it seems harmless, avoiding seems prudent and more future
proof.
Some versions of complete-ant-cmd.pl add "import-project-name." prefix
to imported targets.

Just check that the ones _ant adds are in our completion if
complete-ant-cmd.pl is present, and ignore any others that might be
there.
To follow existing actual convention and "best practice".
Fixes the test failure of `test_10' (test_man.py).

The decorator `@pytest.mark.complete(require_cmd=True)' is referenced
only when the fixture `completion' is requested.  Since `completion'
was not requested by `test_10', `require_cmd=True' was ignored.  Now
`test_10' checks the command (non-)existence on its own for test
skipping.
In the previous version of `bash_env_saved', `bash.cwd' was used to
retrieve the current working directory of the Bash process, but it
turned out that `bash.cwd' just retains the value of the startup time
of the process but not the current one.  Instead we shall store the
real current working directory in a shell variable inside the Bash
process.
`bash_env_saved' now tries to proceed the process of the restoration
of the shell environment even when some of restoration operations fail
in order to reduce later errors caused by incomplete restoration.
In the previous version of `bash_env_saved', when the same variables
are saved or restored in the nested `with bash_env_saved()' statement,
the variables are not correctly restored.  It is not clear whether
there are real instances of such cases in existing tests, but
`bash_env_saved' shall support the nested call of `with
bash_env_saved()' to avoid future troubles.
Users often set `BASH_COMPLETION_USER_FILE=/dev/null' to explicitly
express that there is no user configuration file.  However, in broken
systems, /dev/null may be a regular file that contains random outputs
from arbitary commands whose outputs are discarded.  We shall
explicitly confirm that the path is not `/dev/null' before sourcing
because we do not want to source this unexpected `/dev/null' file.

In fact, this caused a problem in the CI testing on GitHub [1] where
the command history is output to `HISTFILE=/dev/null'.  Here,
`/dev/null' gets the history entry `source bash_completion' and then
sourced from `bash_completion' itself, which results in an infinite
source chain of `bash_completion' -> `/dev/null' -> `bash_completion'
-> `/dev/null' -> ... while invoking random commands from the command
history.

[1] scop#529
[Problem] We have previously set HISTFILE=/dev/null to leave user's
history file alone in the test, but it turned out to break the system.
When the number of history entries reach HISTFILESIZE, Bash tries to
replace the entity at $HISTFILE with a regular file.
HISTFILE=/dev/null causes the removal of the device /dev/null and
creation of a regular file at /dev/null.  This Bash behavior was fixed
in Bash 4.4 after the following bug-bash discussion:

https://lists.gnu.org/archive/html/bug-bash/2015-01/msg00138.html

[Solution] As a workaround, we prepare an empty temporary file for
each test.

[Remark] Another possible solution was to unset HISTFILE in
test/config/bashrc.  However test/config/bashrc is sourced after the
first prompt is shown, i.e., after the user's history file is loaded.
This doesn't necessarily cause problems, but we rather use an empty
file for the history to perform tests in a unqiue condition.
Not enumerating every detail here, will set up commitlint for that, and
have it run in pre-commit.
scop and others added 29 commits December 1, 2021 08:50
The function "_expand" processes "cur" that begins with a tilde, which
is actually already processed by earlier codes in these completions.
Thus "_expand" will never see "cur" starting with a tilde there.

The earlier codes processing the tilde were introduced in commit
1b85a1b.  The calls of "_expand" was moved after the code in commit
add4e3c, for which we forgot the background unfortunately.  We here
just remove the calls of "_expand".
fix(man,info): remove ineffective "_expand" calls
We remove these symbolic links to "test/config/{bashrc,inputrc}".
These contents are so diverged to the testing direction that they are
no longer appropriate for documentation purposes.
Per discussion at
https://asciidoctor.org/docs/asciidoc-recommended-practices/#section-titles

> Keeping the length of the underline in sync with the title length is
> an unnecessary task and time waster. A more substantial reason is that
> the mapping between character and heading level is very difficult to
> remember and not obvious to new AsciiDoc adopters.
Prevents asciidoctor error for it about an invalid part, without a
section.
…cop#674)

The man page is missing bunch of equal signs for FIDO options that take an argument.

Co-authored-by: Koichi Murase <[email protected]>
While at it, make semantics of setting the variable more accurate: it
needs to be set before the *tar* completion is sourced (not
bash_completion), and it needs to be set to a non-null value.
They are largely duplicating what we have in doc/bash_completion.txt,
and inaccurate in saying the vars can be set to anything -- they need to
be set to a non-null value.
To my knowledge, gems are just `*.gem`, not e.g. `*.gem.xz` etc.
Prior to this commit, the current state of `IFS` was stored into a local
variable `ifs`. Later, IFS is restored by setting `IFS=$ifs`. If the
user starts with either a set `IFS` or an empty `IFS`, then this
behavior is correct. If the user starts with an unset `IFS`, then
`IFS=$ifs` results in an empty `IFS`, which is not the original state.
Most of the time an empty `IFS` is not desired, and indeed it breaks
some things inside bash-completions. eg

```
unset IFS
local ifs=$IFS IFS=$' \t\n'
restore=$(shopt -po noglob)
IFS=$ifs
$restore
...
-bash: set -o noglob: command not found
```

With this commit, all instances of `local ifs=$IFS` have been removed.
Instead, the `local IFS` is dynamically unset.
Also add tests for non-hardcoded options and add some options not shown in
"rsync --help".
This removes some ineffective IFS settings IFS=$'\0' found in scop#687
[1].  The quoted strings are not subject to the word splitting, where
IFS does not have effects.  The setting IFS=$'\0' has been first
introduced in 99b2894, and the related codes have been updated in
1c4b461, 2f61acd, 4375c4b, and f241252.  In the change history,
the setting IFS=$'\0' does not seem to have had any effect at any
point, so we just remove the setup IFS=$'\0' today.

The original intent is actually not so clear.  The related code has
been first introduced to process octal escape sequences of the form
`\ooo` (where `o` is a digit 0-7) used in `/etc/fstab` [2].  One
possibile intent might have been to split the filesystem names by NUL
represented by `\000` in `/etc/fstab`, but we cannot find any
information on the special meaning of NUL in `/etc/fstab`.  We think
the intent has probably been to make sure that the word splitting
would not happen, which was actually not needed for the quoted
strings.

[1] scop#687
[2] man fstab (found online at e.g. https://linux.die.net/man/5/fstab)
@andychu
Copy link

andychu commented Feb 20, 2022

Sorry for the delayed response ... I'm not maintaining this repo for public use. (I use it myself and it's been OK for awhile)

There were some patches I had, but I wanted to merge them into upstream, not the opposite

oils-for-unix/oils#717

At the moment, I don't remember what all the changes were, but I'd be happy to discuss if you're interested in this area

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.