Skip to content

add debug option to setup-environment-commands #289

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

Closed
wants to merge 1 commit into from
Closed

Conversation

molleweide
Copy link
Collaborator

@molleweide molleweide commented Feb 21, 2025

  • adds an args processing while loop
  • adds debug flag that allows for detailed logging of env before and after setup-environment-commands. --debug sets a variable that allows env.bash print in detail what happens so that you can diagnose exactly what happens.

I think this is preparatory to moving #288 into SEC so that you can more easilly debug what happens when you are playing around with the cache.

- adds an args processing while loop
- adds debug flag that allows for detailed logging of env before and
  after `setup-environment-commands`
@balupton
Copy link
Member

Interesting. The WIP stuff I'm doing has a __debug_lines helper in bash.bash.

Why --debug here better than debug-bash -x -- setup-environment-commands

@molleweide
Copy link
Collaborator Author

Interesting. The WIP stuff I'm doing has a __debug_lines helper in bash.bash.

Yeah, i know ;)

Why --debug here better than debug-bash -x -- setup-environment-commands

Good question. I realized the set -x exists but i think that sometimes just creates too much output, and if you are unsure of what has happened then you might have to put it in a handful places. So, just passing --debug and then have it print everything in a nice and concise manner seems more useful in some cases.

For example, I had a path with oh-my-zsh that i couldnt understand why it was not being set correctly. Using the --debug flag made it possible to see that "Ahaa, it was already being set before dorothy for some reason and so the debug showed variable 'variable was modified' and now things made sense."

Also, with the history issue we both experience, atm i could not scroll back far enough to check the value in the output. etc etc.

@balupton
Copy link
Member

Makes sense. Let's reevaluate after devilbird merges.

@balupton
Copy link
Member

Had a crazy situation where updating __do to __read_whole instead of $(<...) to preserve trailing newlines caused setup-environment-commands to fail due to there now being a newline value in the PATH.

CleanShot 2025-07-10 at 13 52 30@2x

https://github.com/bevry/dorothy/actions/runs/16187103925/job/45694731505#step:2:2589

The latest WIP code pushed to #281 includes debug code for the environment stuff now. So thank you for the inspiration for this.

Also, I was able to use the new bash.bash helpers to dramatically simplify the env.bash stuff.

@balupton balupton mentioned this pull request Jul 10, 2025
28 tasks
balupton added a commit that referenced this pull request Jul 11, 2025
- ci:
    - remove `macos-13`, deprecated: https://github.blog/changelog/2025-07-11-upcoming-changes-to-macos-hosted-runners-macos-latest-migration-and-xcode-support-policy-updates
- `*`:
    - remove trailing empty line in help texts for consistency
- `echo-numeric`:
    - remove error message, perhaps re-add at a later date with `--verbose`
- `fs-filename`:
    - fix bad `__slice` call
- `checksum`, `cpr`, `fs-path`:
    - remove unnecessary `__slice` call
- `choose`:
    - define array targets, such that references work as intended
- `debug-bash`, `dorothy`, `eval-tester`, `setup-environment-commands`, `bash.bash`:
    - redo and rethink debug handling
- `bash.bash:(__do|__split)`, `dorothy-internals`, `setup-environment-commands`, `eval-tester`, `styles.bash`, `env.bash`:
    - correctly handle trailing newlines
    - `__do` now keeps them by default for consistency with other targets, and `eval-tester` removes them by default for legacy reasons (thousands of test cases would need to be updated)
- `dorothy-internals`:
    - fixed regressed bash binary/version handling
    - test all implementatiosn
    - fix leak of incomplete calculation
    - style `it was` and `it should be` comparisons
    - split into `bash-legacy.bash`, and add new tests for `bash.bash` helpers
- `echo-revolving-door`:
    - fix bad earlier conversion to `__slice` that wasn't needed
- `echo-style`, `bash.bash`, `styles.bash`:
    - split `__is_var_set` into `__is_var_declared` and `__is_var_defined`
- `echo-verbose`, `eval-tester`, `bash.bash:__dump`, `styles.bash`:
    - use the correct `...nothing-provided` style
- `bash.bash`:
    - rename several capability detection variables for consistency
    - add `__print_without_styles`
    - fix `__dump`'s handling of empty, declared, and undefined variables
    - have `__dump` support `--value=<value>`
    - update `BASH_VERSION_LATEST` to `5.3`
    - use more performant array string conversions on bash versions that support them
    - error messages now use `__dump` for better clarity
    - fix bad detection of var capabilities on bash 4.3 and implement workaround
    - `__to`: do not do divergent behaviour across bash versions for undefined variables, instead error requiring definition (see the `choose` changelog entry earlier, and the definitions now in all reference helpers in `bash.bash`)
    - `__to`, `__do`: more performant tty target handling
    - `__do`: support `--(no-)trailing-lines` flag, which was unnecessarily complicated due to bash <4.4 bugs, see <https://gist.github.com/balupton/cb05a7a8a161a9df2b246cf1491b7654>
    - `__do`: remove a trailing newline from a status file write
    - `__do`: if reading the semaphore fails, handle that
    - `__do`: pass `--inverted` to redirection eval calls to avoid (hopefully) unnecessary recursion
    - `__ansi_trim`: improve performance dramatically by simple pattern matching at the start and using substring matching instead of globs for non-pattern keys
    - `__cat_whole` => `__read_whole` which is now used throughout dorothy
    - `__array`: consistent behaviour with other reference helpers
    - fix missing definitions in all `bash.bash` reference helpers causing divergent bash behaviour across bash versions
    - `__slice`: rewrote the length handling for simplicity and performance, and improved error messages when out of bounds
    - `__split`: support `--(no-)trailing-lines` flag for `--invoke` usage
- `is-windows`:
    - document various environment variables
- `setup-util-bash`:
    - move the dev dep handling into the install function so existence checks aren't non-performant
- `setup-environment-commands`, `env.bash`:
    - better debugging, thanks @molleweide for inspiration from #289
    - handling malformated `env` output, because that is possible
    - use the `bash.bash` helpers to dramatically simplify what was once very complex redundant logic
    - use associative arrays on bash versions that support them for performance
    - use a single print at the end for performance
    - use safety functions (todo: use/convert more later)
- `environment.*`, `history.sh`, `oz.fish`:
    - change from `>>/dev/stderr` to `>&2` as all the relavant shells support that conversion
- `environment.sh`:
    - better output and capturing and handling of failures, allowing survival on failures
balupton added a commit that referenced this pull request Jul 11, 2025
- ci:
    - remove `macos-13`, deprecated: https://github.blog/changelog/2025-07-11-upcoming-changes-to-macos-hosted-runners-macos-latest-migration-and-xcode-support-policy-updates
- `*`:
    - remove trailing empty line in help texts for consistency
- `echo-numeric`:
    - remove error message, perhaps re-add at a later date with `--verbose`
- `fs-filename`:
    - fix bad `__slice` call
- `checksum`, `cpr`, `fs-path`:
    - remove unnecessary `__slice` call
- `choose`:
    - define array targets, such that references work as intended
- `debug-bash`, `dorothy`, `eval-tester`, `setup-environment-commands`, `bash.bash`:
    - redo and rethink debug handling
- `bash.bash:(__do|__split)`, `dorothy-internals`, `setup-environment-commands`, `eval-tester`, `styles.bash`, `env.bash`:
    - correctly handle trailing newlines
    - `__do` now keeps them by default for consistency with other targets, and `eval-tester` removes them by default for legacy reasons (thousands of test cases would need to be updated)
- `dorothy-internals`:
    - fixed regressed bash binary/version handling
    - test all implementatiosn
    - fix leak of incomplete calculation
    - style `it was` and `it should be` comparisons
    - split into `bash-legacy.bash`, and add new tests for `bash.bash` helpers
- `echo-revolving-door`:
    - fix bad earlier conversion to `__slice` that wasn't needed
- `echo-style`, `bash.bash`, `styles.bash`:
    - split `__is_var_set` into `__is_var_declared` and `__is_var_defined`
- `echo-verbose`, `eval-tester`, `bash.bash:__dump`, `styles.bash`:
    - use the correct `...nothing-provided` style
- `bash.bash`:
    - rename several capability detection variables for consistency
    - add `__print_without_styles`
    - fix `__dump`'s handling of empty, declared, and undefined variables
    - have `__dump` support `--value=<value>`
    - update `BASH_VERSION_LATEST` to `5.3`
    - use more performant array string conversions on bash versions that support them
    - error messages now use `__dump` for better clarity
    - fix bad detection of var capabilities on bash 4.3 and implement workaround
    - `__to`: do not do divergent behaviour across bash versions for undefined variables, instead error requiring definition (see the `choose` changelog entry earlier, and the definitions now in all reference helpers in `bash.bash`)
    - `__to`, `__do`: more performant tty target handling
    - `__do`: support `--(no-)trailing-lines` flag, which was unnecessarily complicated due to bash <4.4 bugs, see <https://gist.github.com/balupton/cb05a7a8a161a9df2b246cf1491b7654>
    - `__do`: remove a trailing newline from a status file write
    - `__do`: if reading the semaphore fails, handle that
    - `__do`: pass `--inverted` to redirection eval calls to avoid (hopefully) unnecessary recursion
    - `__ansi_trim`: improve performance dramatically by simple pattern matching at the start and using substring matching instead of globs for non-pattern keys
    - `__cat_whole` => `__read_whole` which is now used throughout dorothy
    - `__array`: consistent behaviour with other reference helpers
    - fix missing definitions in all `bash.bash` reference helpers causing divergent bash behaviour across bash versions
    - `__slice`: rewrote the length handling for simplicity and performance, and improved error messages when out of bounds
    - `__split`: support `--(no-)trailing-lines` flag for `--invoke` usage
- `is-windows`:
    - document various environment variables
- `setup-util-bash`:
    - move the dev dep handling into the install function so existence checks aren't non-performant
- `setup-environment-commands`, `env.bash`:
    - better debugging, thanks @molleweide for inspiration from #289
    - handling malformated `env` output, because that is possible
    - use the `bash.bash` helpers to dramatically simplify what was once very complex redundant logic
    - use associative arrays on bash versions that support them for performance
    - use a single print at the end for performance
    - use safety functions (todo: use/convert more later)
- `environment.*`, `history.sh`, `oz.fish`:
    - change from `>>/dev/stderr` to `>&2` as all the relavant shells support that conversion
- `environment.sh`:
    - better output and capturing and handling of failures, allowing survival on failures
@balupton balupton closed this Jul 11, 2025
@balupton balupton deleted the env-debug branch July 11, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants