Refactor: Rename NET_TOR to NET_ONION #13575

pull ghost wants to merge 1 commits into bitcoin:master from changing 7 files +20 −20
  1. ghost commented at 8:57 am on June 30, 2018: none

    This is a follow-up to #13532, where @promag already asked if this renaming would make sense.

    If network shall be named Onion instead of Tor (like in the option onlynet), renaming the network enum NET_TOR to NET_ONION maybe would make sense and be stringent.

    Change was produced with the following script:

    0#!/bin/bash
1
2for file in $(grep --exclude-dir='.git' --files-with-matches --binary-files=without-match --recursive NET_TOR bitcoin/)
3do
4    sed --in-place --expression='s/NET_TOR/NET_ONION/g' $file
5done

    Tor is used at many other places in the code, though.

  2. fanquake added the label Refactoring on Jun 30, 2018
  3. fanquake added the label P2P on Jun 30, 2018
  4. promag commented at 9:15 am on June 30, 2018: member
    Follow https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#scripted-diffs for these changes.
  5. ghost commented at 11:00 am on June 30, 2018: none
    OK I did update the commit message.
  6. fanquake commented at 5:12 am on July 1, 2018: member

    @wodry Have a look at https://github.com/fanquake/bitcoin/commit/454bec264ef9d104a4062992e98fe8ca1513844c for an example of this scripted diff that doesn’t require the for/do loop, grep etc. Ideally, we want to keep the verify scripts as simple and concise as possible.

    For anyone wondering about the -i "", it’s required for sed on macOS if you want to edit files in-place without passing a backup extension.

  7. ghost commented at 1:42 pm on July 1, 2018: none

    @fanquake OK i replaced the script code. Do other versions of sed not understand the s/regexp/replacement/ syntax? I am surprised that one can leave out the first and last /

    The man page for GNU sed 4.4 on Linux says:

    s/regexp/replacement/ Attempt to match regexp against the pattern space. If successful, replace that portion matched with replacement.

  8. fanquake commented at 1:48 pm on July 1, 2018: member

    @wodry Apologies, my commit message was meant to include the first and last /…

    0
1-BEGIN VERIFY SCRIPT-
2sed -i "" 's/NET_TOR/NET_ONION/g' src/init.cpp src/netaddress.cpp src/netaddress.h src/netbase.cpp src/qt/optionsdialog.cpp src/test/netbase_tests.cpp src/torcontrol.cpp
3-END VERIFY SCRIPT-
  9. ghost commented at 2:00 pm on July 1, 2018: none

    Does not work for me:

    bitcoin$ LANG=C sed -i "" ‘sNET_TOR/NET_ONIONg’ src/init.cpp src/netaddress.cpp src/netaddress.h src/netbase.cpp src/qt/optionsdialog.cpp src/test/netbase_tests.cpp src/torcontrol.cpp sed: can’t read sNET_TOR/NET_ONIONg: No such file or directory

    This does also not work:

    bitcoin$ LANG=C sed -i "" ’s/NET_TOR/NET_ONION/g’ src/init.cpp src/netaddress.cpp src/netaddress.h src/netbase.cpp src/qt/optionsdialog.cpp src/test/netbase_tests.cpp src/torcontrol.cpp sed: can’t read s/NET_TOR/NET_ONION/g: No such file or directory

    This does work (no space between -i and “”):

    sed -i"" ’s/NET_TOR/NET_ONION/g’ src/init.cpp src/netaddress.cpp src/netaddress.h src/netbase.cpp src/qt/optionsdialog.cpp src/test/netbase_tests.cpp src/torcontrol.cpp

    This does also work also not work (backup files are created with suffix =) (-i=""):

    bitcoin$ LANG=C sed -i="" ’s/NET_TOR/NET_ONION/g’ src/init.cpp src/netaddress.cpp src/netaddress.h src/netbase.cpp src/qt/optionsdialog.cpp src/test/netbase_tests.cpp src/torcontrol.cpp

  10. fanquake commented at 2:02 pm on July 1, 2018: member
    @wodry See my comment above.
  11. ghost commented at 2:05 pm on July 1, 2018: none
    @fanquake please see my edits. Is sed -i"" 's/NET_TOR/NET_ONION/g' src/init.cpp src/netaddress.cpp src/netaddress.h src/netbase.cpp src/qt/optionsdialog.cpp src/test/netbase_tests.cpp src/torcontrol.cpp ok for Mac?
  12. ghost commented at 2:28 pm on July 1, 2018: none

    What does the Travis error mean. Is something not ok with my commit?

    0The command "if [ "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then test/lint/commit-script-check.sh $TRAVIS_COMMIT_RANGE; fi" exited with 1.
10.12s$ test/lint/git-subtree-check.sh src/crypto/ctaes
2src/crypto/ctaes in HEAD currently refers to tree 1b6c31139a71f80245c09597c343936a8e41d021
3src/crypto/ctaes in HEAD was last updated in commit 8501bedd7508ac514385806e191aec21ee978891 (tree 1b6c31139a71f80245c09597c343936a8e41d021)
4subtree commit 003a4acfc273932ab8c2e276cde3b4f3541012dd unavailable: cannot compare

    I double checked that my change commit is set up onto a freshly fetched master upstream branch.

  13. fanquake commented at 2:31 pm on July 1, 2018: member

    @wodry You need to add “scripted-diff:” to the commit title. i.e “scripted-diff: rename NET_TOR to NET_ONION”

    See scripted diffs in the developer notes.

  14. ghost commented at 2:33 pm on July 1, 2018: none
    Ah tanks, forgot that, fixed.
  15. Empact commented at 7:15 pm on July 1, 2018: member
    Could you apply the scripted-diff to all sources? If that works it will be more robust against independent introduction.
  16. MarcoFalke commented at 11:41 pm on July 1, 2018: member

    git grep -l could be helpful

    On Sun, Jul 1, 2018, 20:16 Ben Woosley notifications@github.com wrote:

    Could you apply the scripted-diff to all sources? If that works it will be more robust against independent introduction.

    — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/bitcoin/bitcoin/pull/13575#issuecomment-401626819, or mute the thread https://github.com/notifications/unsubscribe-auth/AGGmv2Fj3zqrckPvPf6b9D7shiIRB0iBks5uCR_vgaJpZM4U95vJ .

  17. ghost commented at 5:47 am on July 2, 2018: none
    @Empact I agree. My original script that I posted in my first comment does that. @MarcoFalke -l is --files-with-matches which I used in my original script that I posted in my first comment. @fanquake I changed back to my original script but changed --in-place to --in-place'' so it should work on MacOS sed, too.
  18. ghost commented at 7:15 am on July 2, 2018: none
    Could someone please advise howto fix the Travis error? I triple checked to follow scripted diffs developer notes)
  19. ken2812221 commented at 7:31 am on July 2, 2018: contributor

    I would suggest

    sed -i 's/NET_TOR/NET_ONION/g' $(git ls-files -- '*.cpp' '*.h')

    Your script failed because you specify the wrong directory. Not bitcoin/ but .

  20. ghost commented at 8:40 am on July 2, 2018: none
    Thanks @ken2812221 for the fix. Isn’t it better to grep for occurrence of NET_TOR in all non-binary files to see if NET_TOR is maybe also in non-.cpp/.h files? If this is not the case, like here, it’s fine, and as a benefit one knows that fact.
  21. MarcoFalke commented at 8:51 am on July 2, 2018: member
    Please use git grep instead of grep
  22. ghost commented at 9:47 am on July 2, 2018: none
    @MarcoFalke Updated
  23. scripted-diff: Replace NET_TOR with NET_ONION 
    -BEGIN VERIFY SCRIPT-

sed --in-place'' --expression='s/NET_TOR/NET_ONION/g' $(git grep -I --files-with-matches 'NET_TOR')

-END VERIFY SCRIPT-

The --in-place'' hack is required for sed on macOS to edit files in-place without passing a backup extension.
    07c493f2d1
  24. Empact commented at 5:02 pm on July 2, 2018: member
    utACK 07c493f
  25. ken2812221 commented at 11:32 pm on July 3, 2018: contributor
    utACK 07c493f2d1053d6a1c7d0ca1650da7b6f94c8948
  26. sipa commented at 11:36 pm on July 3, 2018: member
    utACK 07c493f2d1053d6a1c7d0ca1650da7b6f94c8948
  27. laanwj commented at 9:50 am on July 4, 2018: member

    utACK 07c493f2d1053d6a1c7d0ca1650da7b6f94c8948

    Tor is used at many other places in the code, though.

    The goal should also not be to replace tor with onion. The distinction is:

    • onion should be used when referring to the representation and connecting to of onion hidden services.
    • tor when dealing with tor in general, so when connecting out through the tor proxy, Tor is special (unlike say, I2P) in that it’s possible to connect both to internal onion addresses and external addresses through exit nodes.
  28. laanwj merged this on Jul 4, 2018
  29. laanwj closed this on Jul 4, 2018
  30. laanwj referenced this in commit 5c05dd6285 on Jul 4, 2018
  31. unknown deleted the branch on Jul 4, 2018
  32. PastaPastaPasta referenced this in commit 237a76f6fe on Apr 16, 2020
  33. PastaPastaPasta referenced this in commit d0d5735290 on Apr 16, 2020
  34. PastaPastaPasta referenced this in commit c62115a12b on Apr 19, 2020
  35. PastaPastaPasta referenced this in commit 1c8012a7dc on Apr 20, 2020
  36. PastaPastaPasta referenced this in commit 14928c3cc9 on May 10, 2020
  37. PastaPastaPasta referenced this in commit dd31a2bb9a on May 12, 2020
  38. PastaPastaPasta referenced this in commit f0e5ef09fc on Jun 9, 2020
  39. PastaPastaPasta referenced this in commit dcd94c323f on Jun 9, 2020
  40. PastaPastaPasta referenced this in commit 732c8d7ab2 on Jun 10, 2020
  41. PastaPastaPasta referenced this in commit 96c5bacba2 on Jun 11, 2020
  42. gades referenced this in commit b83e2d7c74 on Jun 25, 2021
  43. random-zebra referenced this in commit 85f000ecbe on Jul 30, 2021
  44. random-zebra referenced this in commit b4751e10ce on Aug 11, 2021
  45. gades referenced this in commit 0ab6502b05 on Jan 29, 2022
  46. DrahtBot locked this on Feb 15, 2022


ghost promag fanquake Empact MarcoFalke ken2812221 sipa laanwj

Labels
Refactoring P2P

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-23 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me