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

script install_db4.sh check for patch command before continuing to install db4 #23579

Merged
merged 1 commit into from
Nov 25, 2021
Merged

script install_db4.sh check for patch command before continuing to install db4 #23579

merged 1 commit into from
Nov 25, 2021

Conversation

ngara
Copy link
Contributor

@ngara ngara commented Nov 23, 2021

First contribution. I've read the CONTRIBUTING guide and hope I'm doing this correctly, but please kindly point out anything I should do differently.

I found while running the contrib/install_db4.sh patch that it would fail suddenly with "patch: command not found". I'd rather see it fail early before doing any work, allow me to install the patch command, and then run again. (CentOS Linux) Here's a PR proposed to fix it.

error message:

...
db-4.8.30.NC/txn/txn_rec.c
db-4.8.30.NC/txn/txn_region.c
db-4.8.30.NC/txn/txn_stat.c
db-4.8.30.NC/txn/txn_util.c
./contrib/install_db4.sh: line 71: patch: command not found

@katesalazar
Copy link
Contributor

"./contrib/install_db4.sh: line 71: patch: command not found" is not obscure enough to need an improvement. NAK.

@maflcko
Copy link
Member

maflcko commented Nov 23, 2021

Wouldn't it be best to remove this script? We already have depends, which can build bdb4.

@ngara
Copy link
Contributor Author

ngara commented Nov 23, 2021

@katesalazar not looking for an adjustment to the message; only to check dependencies before starting write operations -- at least for new users like myself who may not have everything installed yet.

@ngara
Copy link
Contributor Author

ngara commented Nov 23, 2021

@laanwj
Copy link
Member

laanwj commented Nov 24, 2021

Concept ACK, thanks for your contribution.

Wouldn't it be best to remove this script? We already have depends, which can build bdb4.

The depends framework doesn't work on all the platforms that this script works, IIRC. Think of the BSDs.
Also it's a simple command to run, nothing to figure out. If we want to encourage people to use depends for this we should document a command that builds only berkeleydb and provides it to the build.

@laanwj
Copy link
Member

laanwj commented Nov 25, 2021

Code review and tested ACK 7bb8eb0

@laanwj laanwj merged commit 14fe4db into bitcoin:master Nov 25, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 26, 2021
…before continuing to install db4

7bb8eb0 script install_db4.sh added check for patch command (Nathan Garabedian)

Pull request description:

  First contribution.  I've read the CONTRIBUTING guide and hope I'm doing this correctly, but please kindly point out anything I should do differently.

  I found while running the contrib/install_db4.sh patch that it would fail suddenly with "patch: command not found".  I'd rather see it fail early before doing any work, allow me to install the `patch` command, and then run again.  (CentOS Linux) Here's a PR proposed to fix it.

  error message:
  ```
  ...
  db-4.8.30.NC/txn/txn_rec.c
  db-4.8.30.NC/txn/txn_region.c
  db-4.8.30.NC/txn/txn_stat.c
  db-4.8.30.NC/txn/txn_util.c
  ./contrib/install_db4.sh: line 71: patch: command not found
  ```

ACKs for top commit:
  laanwj:
    Code review and tested ACK 7bb8eb0

Tree-SHA512: f0c59ec509dff6637c41eec2923fe708456c3a7ae04495b12c5492681a1f95d1d9a643a2649df13ba8e6ac708680c627657b6989b62eff63be021e92e1d7c4a8
laanwj added a commit to bitcoin-core/gui that referenced this pull request Dec 9, 2021
…nstall_db4.sh

b062da0 contrib: add check for wget command in install_db4.sh (Florian Baumgartl)

Pull request description:

  This PR is motivated by bitcoin/bitcoin@7bb8eb0 commit (see also bitcoin/bitcoin#23579) and ensures that `install_db4.sh` will check for `curl` and `wget` utilities. Currently, the conditional statement in the `http_get()` function assumes that `wget` is always available but we actually do not know it since there is no check or validation for the `wget` command. So let's make sure that we check for both commands and print an error message if they are missing.

ACKs for top commit:
  jamesob:
    ACK bitcoin/bitcoin@b062da0
  laanwj:
    Tested ACK b062da0
  shaavan:
    ACK b062da0

Tree-SHA512: bfc1ccad9a5b99764b759e02dde1976616c2af4747b7d5af8e71d33624c2cb21d93a09a60d244756e86bbd5fd7541331c62d7eb84d3458b6a059f1d9cb2a5f42
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 10, 2021
…b4.sh

b062da0 contrib: add check for wget command in install_db4.sh (Florian Baumgartl)

Pull request description:

  This PR is motivated by bitcoin@7bb8eb0 commit (see also bitcoin#23579) and ensures that `install_db4.sh` will check for `curl` and `wget` utilities. Currently, the conditional statement in the `http_get()` function assumes that `wget` is always available but we actually do not know it since there is no check or validation for the `wget` command. So let's make sure that we check for both commands and print an error message if they are missing.

ACKs for top commit:
  jamesob:
    ACK bitcoin@b062da0
  laanwj:
    Tested ACK b062da0
  shaavan:
    ACK b062da0

Tree-SHA512: bfc1ccad9a5b99764b759e02dde1976616c2af4747b7d5af8e71d33624c2cb21d93a09a60d244756e86bbd5fd7541331c62d7eb84d3458b6a059f1d9cb2a5f42
RandyMcMillan pushed a commit to RandyMcMillan/mempool-tab that referenced this pull request Dec 23, 2021
…nstall_db4.sh

332d756 contrib: add check for wget command in install_db4.sh (Florian Baumgartl)

Pull request description:

  This PR is motivated by bitcoin/bitcoin@917d12d commit (see also bitcoin/bitcoin#23579) and ensures that `install_db4.sh` will check for `curl` and `wget` utilities. Currently, the conditional statement in the `http_get()` function assumes that `wget` is always available but we actually do not know it since there is no check or validation for the `wget` command. So let's make sure that we check for both commands and print an error message if they are missing.

ACKs for top commit:
  jamesob:
    ACK bitcoin/bitcoin@332d756
  laanwj:
    Tested ACK 332d756
  shaavan:
    ACK 332d756

Tree-SHA512: bfc1ccad9a5b99764b759e02dde1976616c2af4747b7d5af8e71d33624c2cb21d93a09a60d244756e86bbd5fd7541331c62d7eb84d3458b6a059f1d9cb2a5f42
@bitcoin bitcoin locked and limited conversation to collaborators Nov 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants