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

fix: resolve some issues with install.sh #188

Merged
merged 2 commits into from
Dec 8, 2021

Conversation

mozzieongit
Copy link
Contributor

@mozzieongit mozzieongit commented Sep 25, 2021

Some people don't use sudo or have curl, sed or wget installed by default.
This adds a check, if sudo, curl and sed are installed and changes wget
to curl as this is the mostly used command in the script. If sudo is not
installed it uses su.

The printf was missing a newline at the end, which resulted in the eval
line being appended directly to the end of the previous line. This is
similar to the problem from #56.

Closes: #175

@mozzieongit
Copy link
Contributor Author

I should probably wait for the dependabot PRs to be merged.

@mozzieongit mozzieongit marked this pull request as ready for review September 25, 2021 14:35
@mozzieongit mozzieongit marked this pull request as draft September 25, 2021 15:05
@mozzieongit mozzieongit changed the title feat: add command checks to install script fix: resolve some issues with install.sh Sep 25, 2021
@mozzieongit
Copy link
Contributor Author

mozzieongit commented Sep 25, 2021

Why does cargo test -p atuin-common --lib suddenly fail? It failed on my machine, too.

But now it works again. I just re-ran it.
Weird.

EDIT:
I ran it multiple times now. It randomly succeeds or fails at different tests. (Even with other commit states)

@ellie
Copy link
Member

ellie commented Sep 29, 2021

Could you share the logs from the failures please? Maybe some of the env var stuff is being non deterministic

@mozzieongit
Copy link
Contributor Author

mozzieongit commented Sep 29, 2021

    Finished test [unoptimized + debuginfo] target(s) in 0.07s
     Running unittests (target/debug/deps/atuin_common-284a7ca8554b08af)

running 4 tests
test utils::tests::test_config_dir ... ok
test utils::tests::test_config_dir_xdg ... ok
test utils::tests::test_data_dir_xdg ... ok
test utils::tests::test_data_dir ... FAILED

failures:

---- utils::tests::test_data_dir stdout ----
thread 'utils::tests::test_data_dir' panicked at '$HOME not found: NotPresent', atuin-common/src/utils.rs:37:38
stack backtrace:
   0: rust_begin_unwind
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/panicking.rs:92:14
   2: core::result::unwrap_failed
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/result.rs:1599:5
   3: core::result::Result<T,E>::expect
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/result.rs:1241:23
   4: atuin_common::utils::home_dir
             at ./src/utils.rs:37:16
   5: atuin_common::utils::data_dir::{{closure}}
             at ./src/utils.rs:49:26
   6: core::result::Result<T,E>::map_or_else
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/result.rs:806:23
   7: atuin_common::utils::data_dir
             at ./src/utils.rs:48:20
   8: atuin_common::utils::tests::test_data_dir
             at ./src/utils.rs:90:20
   9: atuin_common::utils::tests::test_data_dir::{{closure}}
             at ./src/utils.rs:87:5
  10: core::ops::function::FnOnce::call_once
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/ops/function.rs:227:5
  11: core::ops::function::FnOnce::call_once
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.


failures:
    utils::tests::test_data_dir

test result: FAILED. 3 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

error: test failed, to rerun pass '-p atuin-common --lib'

EDIT: Added Backtrace

Some people don't use sudo or have curl, sed or wget installed by default.
This adds a check, if sudo, curl and sed are installed and changes wget
to curl as this is the mostly used command in the script. If sudo is not
installed it uses su.

Closes: atuinsh#175
The printf was missing a newline at the end, which resulted in the eval
line being appended directly to the end of the previous line.
@mozzieongit
Copy link
Contributor Author

Yay. The test finished successfully.

@mozzieongit mozzieongit marked this pull request as ready for review December 8, 2021 13:53
@ellie ellie enabled auto-merge (squash) December 8, 2021 23:21
Copy link
Member

@ellie ellie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thank you for your work here! 💖

@ellie ellie merged commit c8f60b2 into atuinsh:main Dec 8, 2021
@prbond
Copy link

prbond commented Dec 9, 2021

Thanks

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.

add a check if sudo is installed
3 participants