Bug 1026473

these two unit tests appear to test for this very use case.

org.rhq.core.util.updater.DeployerTest.baseUpdateTest(boolean, boolean)
org.rhq.plugins.ant.AntBundlePluginComponentTest.upgrade(boolean)

Can you attach the exact bundle distro file you are using?

(as per libor - the replication step #2 includes putting a file in /tmp/foo/subdir2)

Created attachment 819742 [details]
bundle version1

Here's my test that fails

on line 71

https://github.com/RedHatQE/jon-tests/blob/master/clitest/src/test/java/com/redhat/qe/jon/clitest/tests/bundles/BundleComplianceTest.java#L63

and after manual check it looks like files are removed but empty directories survive bundle upgrade.

(In reply to Libor Zoubek from comment #3)
> Here's my test that fails
> 
> on line 71
> 
> https://github.com/RedHatQE/jon-tests/blob/master/clitest/src/test/java/com/
> redhat/qe/jon/clitest/tests/bundles/BundleComplianceTest.java#L63
> 
> and after manual check it looks like files are removed but empty directories
> survive bundle upgrade.

Right, OK, then I think this behavior hasn't changed and is working as before. The bundle upgrade stuff cares about file content, not directories. As long as the files are gone after an upgrade, that means we did the right thing. I believe bundle upgrading allows for created directories (that were created after the installation) to remain, though they are empty. Empty directories we do not delete after an upgrade. I'm pretty sure nothing changed here from earlier RHQ versions, but I'm not 100% sure of that. In any case, there is no real deterimental effects that I know of if the directories are left empty.

I agree, keeping empty dirs is harmless, but at least we should mention it in docs, that we do not clean up empty dirs. I'll udpate my tests not to assert empty dirs

I don't think documentation is the way to go. That would only confuse the issue and be the stand-in bandage approach.

From a user perspective the last thing I want to see is:

   compliance = full except in cases where its not


Furthermore, the existence of these directories can lead to confusion. For example, if a conf or META-INF directory gets left behind which could optionally contain some other file. A user may add that some other file without giving it another thought. The fact that after an upgrade the directory remains intact, would leave one to believe that all is well. However, due to compliance = full and our out-of-band documented exception explaining that compliance = full != full, the loss of the data in the directory may not be noticed right away. This resulting in loss of time and resources.

Considering directories are treated as files on all support operating systems, I am not sure why we can't just handle this correctly. Is there a reason we avoid removal of a directory entry because it is not of a specific type?

I have dropped the severity to low and updated the title as comment 4 does indicate that this issue is cosmetic or a usability issue only and not functionally impacting.

Re-targeted to CP02 to reduce CP01 overall payload.

in master

commit d306411243c361b1fb3a009e6d959447322c3b5f
Author: Libor Zoubek <lzoubek>
Date:   Fri Feb 14 16:49:55 2014 +0100

    [BZ 1026473] When upgrading bundle with compliance=full deployment folder
    still contains empty directories not found in bundle

    This patch adds support for handling directories in bundle-deployment client
    side. The impact is only for doing updrade or revert (not initial deploy).
    FileHashcodeMap.rescan now reports every single directory with special HASH.
    Then in Deployer code we need to fill it directories to 2 other maps
    (original=from hashcodes.dat, newFiles=from bundle content). Then when
    processing diff we have to sort items to be backed up/deleted and process
    them in reverse order. (Then if there is 'dir/file', then 'dir/file' is
    processed before 'dir'). Finally backup for dir is just creating it in
    backup directory. Several tests have been fixed - especially directories now
    appear in diff additions together with new files. SimpleDeployerTest has
    been updated to test empty dir handling.

in 3.2.x

commit 79814ab96769a1227d616d49d3c38a9a0a76caef
Author: Libor Zoubek <lzoubek>
Date:   Fri Feb 14 16:49:55 2014 +0100

(cherry picked from commit d306411243c361b1fb3a009e6d959447322c3b5f)

Signed-off-by: John Mazzitelli <mazz>

Previous commit caused regression in ant-bundle plugin tests.

Now fixed in master

commit e9b330e8597c6607d4c6fa3126a6c6cb9148364d
Author: Libor Zoubek <lzoubek>
Date:   Tue Mar 4 10:32:43 2014 +0100

    [BZ 1026473] When upgrading bundle with compliance=full deployment folder

    As we now handle directories in bundle, directory name 'test' wouldn't match
    pattern 'test/.*' (which is 'rhq:ignore test/**'). Fixed by adding trailing
    slash when evaluating match.

branch:  release/jon3.2.x
link:    https://github.com/rhq-project/rhq/commit/f53570725
time:    2014-03-04 10:48:28 +0100
commit:  f53570725a030fc2a883f395ebd3c93ca7ef1f00
author:  Libor Zoubek - lzoubek
message: [BZ 1026473] When upgrading bundle with compliance=full deployment
         folder

Moving to 3.2.1 as this has already been fixed and cherry-picked to the release branch.

Moving to ON_QA as available for testing in the following brew build:
https://brewweb.devel.redhat.com//buildinfo?buildID=340294

Note: the installed version is still JON 3.2.0.GA by design and this represents part of the payload for JON 3.2.1 also known as cumulative patch 1 for 3.2.0.GA.  How this will be delivered to customers is still being discussed.

Verified on:
Version :	
3.2.0.GA
Build Number :	
d18651a:f535707

JON 3.2.1 released week of 5/5/2014