doc: Add some better examples for scripted diff #17411

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2019_11_scripted_diff_examples changing 1 files +31 −2
  1. laanwj commented at 8:12 AM on November 8, 2019: member

    The current example isn't too great, for example it uses find instead of git ls-files. Add a subsection with suggestions and examples.

    Feel free to propose some other great examples to add.

  2. laanwj added the label Docs on Nov 8, 2019
  3. in doc/developer-notes.md:941 in d8ae24fae2 outdated
     936 | +does a global replacement but excludes certain directories.
     937 | +
     938 | +To find all previous uses of scripted diffs in the repository, do:
     939 | +
     940 | +```
     941 | +git log --grep="-BEGIN VERIFY SCRIPT-"
    


    promag commented at 8:35 AM on November 8, 2019:

    Great tip!

  4. promag commented at 8:38 AM on November 8, 2019: member

    Concept ACK.

    You have to update the TOC.

  5. practicalswift commented at 9:42 AM on November 8, 2019: contributor

    Concept ACK -- git ls-files and git grep are both great! :)

  6. MarcoFalke approved
  7. laanwj commented at 6:03 PM on November 8, 2019: member

    BTW: what is the best way to do a re-apply of a conflicted scripted diff when rebasing? I did the following to rebase #17410 and it seems it could be more efficient, and if so, we'd want to add it to the documentation:

    $ git rebase master
    First, rewinding head to replay your work on top of it...                                      
    Applying: Rename `db` log category to `walletdb` (like `coindb`)                                          
    Applying: scripted-diff: Change `BCLog::DB` to `BCLog::WALLETDB`                                          Using index info to reconstruct a base tree...                                                            
    M       src/wallet/db.cpp                                                                          
    Falling back to patching base and 3-way merge...                                        
    Auto-merging src/wallet/db.cpp                                                                 
    CONFLICT (content): Merge conflict in src/wallet/db.cpp                         
    error: Failed to merge in the changes.                                                                    
    Patch failed at 0002 scripted-diff: Change `BCLog::DB` to `BCLog::WALLETDB`               
    Use 'git am --show-current-patch' to see the failed patch               
                                                                                                              
    Resolve all conflicts manually, mark them as resolved with                                 
    "git add/rm <conflicted_files>", then run "git rebase --continue".                             
    You can instead skip this commit: run "git rebase --skip".                                                
    To abort and get back to the state before "git rebase", run "git rebase --abort". 
    

    do hard reset, to get rid of git's attempt at re-applying, and conflict markers

    $ git reset --hard
    

    manually paste script steps

    $ git grep -l "BCLog::DB" src | xargs sed -i "s/BCLog::DB/BCLog::WA
    LLETDB/g"
    $ sed -i "s/DB          =/WALLETDB    =/g" src/logging.h
    $ git status
     M src/logging.cpp                            
     M src/logging.h                               
     M src/wallet/db.cpp                              
     M src/wallet/walletdb.cpp
    

    mandually git add changed objects, and roll rebase forward

    $ git add src/logging.cpp src/logging.h src/wallet/db.cpp src/wallet/walletdb.cpp
    $ git rebase --continue
    
  8. ryanofsky commented at 7:27 PM on November 8, 2019: member

    I have a one-line script-get bash function would let you replace the manual paste step:

    bash -c "$(script-get HEAD)"
    

    Probably more of this could be scripted in general, though

  9. laanwj commented at 7:35 PM on November 8, 2019: member

    Thanks!

    Indeed, my idea was that this is pretty much an alternative merge strategy: ignore git's own attempt (which could cause conflicts or miss new occurences) in favor of re-running the script. It won't have any merge conflicts, by definition, though obviously the result still has to be reviewed.

  10. in doc/developer-notes.md:914 in d8ae24fae2 outdated
     909 | @@ -910,7 +910,35 @@ For development, it might be more convenient to verify all scripted-diffs in a r
     910 |  test/lint/commit-script-check.sh origin/master..HEAD
     911 |  ```
     912 |  
     913 | -Commit [`bb81e173`](https://github.com/bitcoin/bitcoin/commit/bb81e173) is an example of a scripted-diff.
     914 | +### Suggestions and examples
    


    hebasto commented at 11:53 AM on November 9, 2019:

    nit: TOC could be updated as @promag suggests.


    laanwj commented at 1:11 PM on November 10, 2019:

    yes, will update the TOC

  11. in doc/developer-notes.md:922 in d8ae24fae2 outdated
     918 | +
     919 | +For efficient replacement scripts, reduce the selection to the files that potentially need to be modified, so for
     920 | +example, instead of a blanket `git ls-files src | xargs sed -i s/apple/orange/`, use
     921 | +`git grep -l apple src | xargs sed -i s/apple/orange/`.
     922 | +
     923 | +Also, it's good to keep the selection of files as specific as possible — for example, replace only in directories where
    


    hebasto commented at 11:54 AM on November 9, 2019:

    nit: contractions, "it's", should be avoided in well-styled docs ;)


    laanwj commented at 1:11 PM on November 10, 2019:

    ok

  12. hebasto approved
  13. hebasto commented at 11:54 AM on November 9, 2019: member

    ACK d8ae24fae2fce7f39a9eb7fa7c9b21fac79e82ac, I have reviewed the code and it looks OK, I agree it can be merged.

  14. doc: Add some better examples for scripted diff
    The current example isn't too great, for example it uses `find` instead
    of `git ls-files`. Add a subsection with suggestions and examples.
    adbe155047
  15. laanwj force-pushed on Nov 19, 2019
  16. laanwj commented at 2:15 PM on November 19, 2019: member

    Updated toc, replaced occurrences of "it's" with "it is".

  17. hebasto approved
  18. hebasto commented at 2:20 PM on November 19, 2019: member

    re-ACK adbe15504713ddba6e9c024c59d977675d49e350

  19. laanwj referenced this in commit 92db280817 on Nov 19, 2019
  20. laanwj merged this on Nov 19, 2019
  21. laanwj closed this on Nov 19, 2019

  22. PastaPastaPasta referenced this in commit ab92bdcfea on Jun 27, 2021
  23. PastaPastaPasta referenced this in commit 947f8cc0c8 on Jun 28, 2021
  24. PastaPastaPasta referenced this in commit 7332fdf9df on Jun 29, 2021
  25. PastaPastaPasta referenced this in commit f9662e2a04 on Jul 1, 2021
  26. PastaPastaPasta referenced this in commit 027cc653bb on Jul 1, 2021
  27. PastaPastaPasta referenced this in commit 41aba0a27a on Jul 14, 2021
  28. DrahtBot locked this on Dec 16, 2021

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: 2026-04-13 15:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me