git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] filter-branch fix and tests

From
Michal Sojka <sojkam1@fel.cvut.cz>
Date
Jan 27, 2010, 23:41 UTC
Message-ID
<201001280041.23182.sojkam1@fel.cvut.cz>
In-Reply-To
<4B606748.2050209@viscovery.net>
On Wednesday 27 of January 2010 17:18:16 Johannes Sixt wrote:
Show 7 quoted lines
> Michal Sojka schrieb:
> > +orig_head=`git show-ref --hash HEAD`
> > +export orig_head
> 
> You place the tree filter in double-quotes. Hence, orig_head will be
> interpolated on the filter-branch command line. You don't need to export
>  it.
You're right. Fixed.
 
Show 10 quoted lines
> > +test_expect_success 'rewrite submodule with another content' '
> > +	git filter-branch --tree-filter "test -d submod && {
> > +					 rm -rf submod &&
> > +					 git rm -rf --quiet submod &&
> > +					 mkdir submod &&
> > +					 : > submod/file
> > +					 } || : &&
> > +					 test $orig_head != `git show-ref --hash HEAD`"
> 
> What is the purpose of the check in the last line?
It should check that something was rewritten, but it was incorrectly put into 
the filed. Fixed.
 
> As long as you have another command after the "} || : &&", you can just
> write "}" instead.
OK.
Show 15 quoted lines
> > +test_expect_failure 'checkout submodule during rewrite' '
> > +	git reset --hard original &&
> > +	git filter-branch -f --tree-filter \
> > +	    "git submodule update --init &&
> > +	     cd submod &&
> > +	     git reset --hard origin/master" HEAD
> 
> You must not change the directory without changing back. Use a sub-shell.
> 
> I'm not sure whether it's worth catering for this use-case anyway.
> Replacing a submodule commit should really be done only in the
> --index-filter. The tree that --tree-filter checks out is intended only as
> a temporary scratch area. It is not intended as a full worktree. In
> particular, since 'submodule update --init' changes the configuration, it
> is extremly dangerous to call from a filter.

I fully agree. I don't plan to put this test in the final version of the patch. I wrote this test because I didn't exactly know which issue has Dscho in mind. If it was this one, I wanted to show that this is not relevant.

I'm sending corrected version of the patch with tests.

Thanks Michal

Previous: Johannes SixtNext: Michal Sojka
Message 6 of 15 in “filter-branch fix and tests”
  1. Michal SojkaJan 25, 2010
  2. 1/2 filter-branch: Fix to allow replacing submodules with another contentMichal Sojka, Jan 25, 2010
  3. 2/2 filter-branch: Add tests for submodulesMichal Sojka, Jan 25, 2010
  4. Michal SojkaJan 27, 2010
  5. Johannes SixtJan 27, 2010
  6. Michal SojkaJan 27, 2010
  7. [PATCHv3] filter-branch: Add tests for submodulesMichal Sojka, Jan 27, 2010
  8. Junio C HamanoJan 28, 2010
  9. Michal SojkaJan 28, 2010
  10. 1/2 filter-branch: Fix to allow replacing submodules with another contentMichal Sojka, Jan 28, 2010
  11. 2/2 filter-branch: Add tests for submodules in tree-filterMichal Sojka, Jan 28, 2010
  12. Junio C HamanoJan 28, 2010
  13. Michal SojkaJan 29, 2010
  14. 1/2 filter-branch: Fix to allow replacing submodules with another contentMichal Sojka, Jan 29, 2010
  15. 2/2 filter-branch: Add tests for submodules in tree-filterMichal Sojka, Jan 29, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.