Renaming a wiki page with attachments does not delete the old name attachment folder

[Francesca Mazzoni <francesca.mazzoni@…>]

I'm on Debian stable with Python 2.7.3 and installed from Debian package.

When I rename a Wiki page that contains attachments, the (empty) folder in which the attachments were stored before the page renaiming is still in the file system.

Steps to reproduce:

1) Create a new wiki page (say MyOldPage), add an attachment (say MyAttach.tgz). The attached file is stored in <trac-env>/attachments/wiki/MyOldPage/MyAttach.tgz.

2) Rename MyOldPage to MyNewPage.

The attachments are now (correctly) stored in <trac-env>/attachments/wiki/MyNewPage/.

I still have the <trac-env>/attachments/wiki/MyOldPage folder.

I think that the folder should be removed, since it is empty.

[Jun Omae]

Thanks for the report. Reproduced on 0.12-stable. The following patch would fix it.

trac/attachment.py

@@ -348 +348 @@
         def do_reparent(db):
             for attachment in list(cls.select(env, parent_realm, parent_id,
                                               db)):
-                attachment_dir = os.path.dirname(attachment.path)
+                attachment_dir[0] = os.path.dirname(attachment.path)
                 attachment.reparent(new_realm, new_id)
         if attachment_dir[0]:
             try:

I'll push the patch with unit test on 0.12.7.

[Jun Omae]

This issue occurs on only 0.12.x. Milestone 0.12.7 for only security fixes. But this issue is not a security issue. Should we fix it? or leave it?

[Ryan J Ollos]

My feeling is that it is really up to you, particularly since we've committed to at least one more release from the 0.12-stable branch. I'd probably go ahead and push the fix though if I had any use for 0.12.x.

After we do the 0.12.7 release though, if we are treating it as a security-fix only branch then future releases will only be as needed.

[Jun Omae]

Thanks for the comments. Fixed in [13610] and recorded only in [13611-13612].

[Ryan J Ollos]

Strangely, trunk​ still shows [13610] from the 0.12-stable branch as eligible for merge. The record-only merges look correct. I guess that must be a bug in Trac?

[Jun Omae]

Subversion client shows the same. I don't think that is a bug.

$ svn mergeinfo --show-revs eligible ^/branches/0.12-stable ^/trunk
r13610

Seems the revision should be recorded from 0.12-stable.

$ svn merge -c13610 --record-only ^/branches/0.12-stable

[Ryan J Ollos]

Maybe it is an issue with Subversion client then, or the way in which the merge was done. Based on previous cases I've seen, it seems like merging [13611] would "carry-over" the record-only of [13610]. Here are some examples: [13155], [13127], [13121], [13091] (more).

I'm not sure what is different in this case, compared to [13090]/[13091]:

$ svn diff -c 13090 https://svn.edgewall.org/repos/trac
Index: branches/1.0-stable
===================================================================
--- branches/1.0-stable (revision 13089)
+++ branches/1.0-stable (revision 13090)

Property changes on: branches/1.0-stable
___________________________________________________________________
Modified: svn:mergeinfo
   Merged /branches/0.12-stable:r13089
$ svn diff -c 13091 https://svn.edgewall.org/repos/trac
Index: trunk
===================================================================
--- trunk       (revision 13090)
+++ trunk       (revision 13091)

Property changes on: trunk
___________________________________________________________________
Modified: svn:mergeinfo
   Merged /branches/1.0-stable:r13090
   Merged /branches/0.12-stable:r13089

My usual process is to run this command:

$ svn merge -c 13090 --record-only ^/branches/1.0-stable trac-trunk

$ svn --version
svn, version 1.8.8 (r1568071)
   compiled Aug 13 2014, 17:12:39 on x86_64-pc-linux-gnu

Copyright (C) 2013 The Apache Software Foundation.
This software consists of contributions made by many people;
see the NOTICE file for more information.
Subversion is open source software, see http://subversion.apache.org/

The following repository access (RA) modules are available:

* ra_svn : Module for accessing a repository using the svn network protocol.
  - with Cyrus SASL authentication
  - handles 'svn' scheme
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' scheme
* ra_serf : Module for accessing a repository via WebDAV protocol using serf.
  - using serf 1.3.3
  - handles 'http' scheme
  - handles 'https' scheme

[Jun Omae]

My usual process is to run this command:

$ svn merge -c 13090 --record-only ^/branches/1.0-stable trac-trunk

Yes. After I committed [13609], I've merged [13609] to 1.0-stable and [13610] to trunk as record-only like this.

$ svn merge -c13609 --record-only ^/branches/0.12-stable branches/1.0-stable
$ svn ci
....
Committed revision 13610.
$ svn merge -c13610 --record-only ^/branches/1.0-stable trunk
$ svn ci
....
Committed revision 13611.

I investigated that.

$ svn --version | head -1
svn, version 1.6.17 (r1128011)
$ svnadmin create svntest
$ svn co file://$PWD/svntest svntest.wc
Checked out revision 0.
$ cd svntest.wc
$ mkdir trunk branches branches/{0.12,1.0}-stable
$ svn add trunk branches
A         trunk
A         branches
A         branches/1.0-stable
A         branches/0.12-stable
$ svn ci -q -m 'testing'
$ touch branches/0.12-stable/test.txt
$ svn add branches/0.12-stable/test.txt
A         branches/0.12-stable/test.txt
$ svn ci -q -m testing
$ svn up
At revision 2.
$ svn merge -c2 --record-only branches/0.12-stable branches/1.0-stable
$ svn ci -q -m merging
$ svn up
At revision 3.

Revision 3 is record-only merge.

Merging this revision as record-only, as the result, we get only changes on 1.0-stable in svn:mergeinfo. That's unexpected result.

$ svn merge -c3 --record-only ^/branches/1.0-stable trunk
$ svn diff

Property changes on: trunk
___________________________________________________________________
Added: svn:mergeinfo
   Merged /branches/1.0-stable:r3

Instead, merging without record-only, we would get changes on 0.12-stable and 1.0-stable in svn:mergeinfo. That's expected result.

$ svn revert -R .
Reverted 'trunk'
$ svn merge -c3 ^/branches/1.0-stable trunk
--- Merging r3 into 'trunk':
 U   trunk
$ svn diff

Property changes on: trunk
___________________________________________________________________
Added: svn:mergeinfo
   Merged /branches/1.0-stable:r3
   Merged /branches/0.12-stable:r2

Therefore, we should merge without record-only for merging changes of record-only merge.

[Christian Boos]

Replying to jomae:

Merging this revision as record-only, as the result, we get only changes on 1.0-stable in svn:mergeinfo. That's unexpected result.

Well, unintuitive perhaps, but that's a perfectly reasonable behavior: with --record-only you only manipulate the target's svn:mergeinfo related to the specified source path and you tell it to not apply any other changes (changes to files, properties, including changes to other paths in the svn:mergeinfo property itself).

Therefore, we should merge without record-only for merging changes of record-only merge.

Yep, that's the way. It's a common gotcha here at work and the source of endless fun ;-)