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

Use URL variables and small enhancements #162

Merged

Conversation

epiccurious
Copy link
Collaborator

@epiccurious epiccurious commented May 24, 2024

Re-create the changes from this PR, including:

  • Use variables for the URLs instead of hard-coding in commands
  • Use array for Bitcoin Core source domains (.onion and bitcoincore.org) for downloading tarball, checksum, and signatures
  • ShellCheck lint changes
  • Improve some comments (capitalize and 2nd-person imperative tense)

@epiccurious epiccurious marked this pull request as draft May 24, 2024 11:27
@epiccurious
Copy link
Collaborator Author

Can we remove these commented lines from install-core?

# # Update $PATH enironment variable to include new user bin
# # shellcheck source=/dev/null
# . $HOME/.profile

@epiccurious
Copy link
Collaborator Author

@BenWestgate Pulled in all the changes from my 73-dont-hardcode-urls-epic fork branch.

Next step is to test the changes, then I'll set the PR ready to review.

@BenWestgate
Copy link
Owner

Can we remove these commented lines from install-core?

# # Update $PATH enironment variable to include new user bin
# # shellcheck source=/dev/null
# . $HOME/.profile

I don't think so. Since Tails 6 there has been a problem with $PATH automatically updating (before a restart) after ~/.local/bin has been created... I uncommented them in the latest 44-Trust-Individuals to make bails-wallet launch again. But I'm still not sure if they're needed.

@BenWestgate BenWestgate added bug Something isn't working enhancement New feature or request labels May 24, 2024
@BenWestgate
Copy link
Owner

Closes #73.

This was linked to issues May 24, 2024
@epiccurious
Copy link
Collaborator Author

The script is stalling, possibly on the guix.sigs step.

Here's the logs:

pastebin.com/cETm7xuM

@BenWestgate
Copy link
Owner

It's getting stuck from thisssssss:
cd bitcoin-core-"$NEW_VER" || {

NEW_VER is assigned in the sub-shell () so it's not available outside of that so it doesn't open up the new directory.

I am going to do that thing with named pipes I told you to fix it. Unless you want to try the new skill.

Copy link
Owner

@BenWestgate BenWestgate left a comment

Choose a reason for hiding this comment

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

I fixed the merge conflict and also resolved the process communication by backgrounding setup-persistence, that way the keys import runs in the main thread.

I also got rid of the Tor fallback since its been down for a year. I also don't use recursive rsync I query for the lastest version and set it in a constant.

Should be easier to read this way.

bails/.local/bin/install-core Outdated Show resolved Hide resolved
@BenWestgate
Copy link
Owner

BenWestgate commented Jun 15, 2024

image
Here's the cause of the downloads overwriting the screen space messily.

I made a file called wgets with two backgrounded downloads that run simultaneously.

when those commands are run in the main terminal, their output is suppressed to log files.

But when they are run in a subshell it stops suppressing them. This is a wget bug I think or a limitation of linux, somehow.

image

You can see the difference when the file is sourced and runs in the main process, that may be what I have to do. I'll have to check install-core does not change any variables that will mess up the rest of b and then they should be able to be suppressed again.

@BenWestgate
Copy link
Owner

@epiccurious: I've cleaned up the terminal output quite a bit and it behaves how you expected now.

Take a look when you get a chance. I also found and protected against a couple race conditions that were previously causing "download" failures more often than they actually happened.

Learned quite a bit today.

@BenWestgate
Copy link
Owner

@epiccurious: For the -choosedatadir window from Bitcoin Core, do you want me to recreate a simplified substitute of it in zenity with the following? Like this?

image
image
image

@BenWestgate
Copy link
Owner

@epiccurious: For the -choosedatadir window from Bitcoin Core, do you want me to recreate a simplified substitute of it in zenity with the following? Like this?

image image image

image

@epiccurious epiccurious marked this pull request as ready for review June 17, 2024 11:26
Copy link
Owner

@BenWestgate BenWestgate left a comment

Choose a reason for hiding this comment

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

LGTM

@BenWestgate BenWestgate merged commit 9239087 into BenWestgate:master Jun 19, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
2 participants