bugGNU Octave - Bugs: bug #43431, gzip does not handle directories...

 
 

bug #43431: gzip does not handle directories and files in different directories

Submitter:  Rik <rik5>
Submitted:  Fri 17 Oct 2014 05:42:28 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 24 Jul 2016 01:33:52 PM UTC, comment #7: 

This changeset also adds a bunch of new tests. Some of those tests are failing but it's an issue with bunzip2 and gunzip that were already present, just not reported and without tests. This new tests only bring them up but the failures themselves are not caused by this change:

http://hg.savannah.gnu.org/hgweb/octave/rev/766f934db568#l3.642

http://hg.savannah.gnu.org/hgweb/octave/rev/766f934db568#l3.674

I have them already reported as:

  • bug #48597 bunzip2 and gunzip remove original files
  • bug #48598 bunzip2 and gunzip do not actually output filenames


Carnë Draug <carandraug>
Group Member
Sat 23 Jul 2016 11:01:18 PM UTC, comment #6: 

I'm seeing three failures after this changeset:


>>>>> processing /scratch/jwe/build/octave/libinterp/dldfcn/xzip.cc-tst
***** test run_test_function (@test_z_z)
!!!!! test failed
ASSERT errors for:  assert (filelist,{z_z_file})

  Location  |  Observed  |  Expected  |  Reason
     .          O(0x0)       E(1x1)      Dimensions don't match
***** test run_test_function (@test_xzip_dir)
!!!!! test failed
ASSERT errors for:  assert (z {2} (z_files {idx}),fpaths {idx})

  Location  |  Observed  |  Expected  |  Reason
     .            O       /tmp/oct-MtWGbS/test1   Expected string, but observed cell
***** test run_test_function (@test_save_to_dir)
!!!!! test failed
ASSERT errors for:  assert (z {2} (z_file),uz_file)

  Location  |  Observed  |  Expected  |  Reason
     .            O       /tmp/oct-si9gqY/test-file   Expected string, but observed cell


John W. Eaton <jwe>
Group administrator
Fri 22 Jul 2016 04:41:31 PM UTC, comment #5: 

Pushed http://hg.savannah.gnu.org/hgweb/octave/rev/766f934db568 which also adds a bunch more tests.

Carnë Draug <carandraug>
Group Member
Thu 21 Jul 2016 06:00:03 PM UTC, comment #4: 

This patch reimplements gzip and bzip2 in C++ making use of the zlib and bzip2 libraries instead of calls to the applications.

I have also added more tests.

(file #37978)

Carnë Draug <carandraug>
Group Member
Sun 19 Jun 2016 09:19:42 PM UTC, comment #3: 

Part of the issues here are shared with bzip2. I wrote a C++ function that makes use of bzip2 library. Hopefully, implementing gzip will be as simple.

This function takes a source filepath and destination source path as arguments. The logic of how to behave with directories still needs to be decided.

(file #37527)

Carnë Draug <carandraug>
Group Member
Sun 19 Jun 2016 01:37:47 AM UTC, comment #2: 

Here's a test that I wrote but I'm not pushing because it's failing and I'm not sure we can easily work around them using the system's gzip and bzip2.  I have no idea what behaviour this things should have, but as they are, it's definetely wrong, and Matlab's behaviour is mad too.


%!test
%! owd = pwd ();
%! outdir = tempname ();
%! testdir = tempname ();
%! unwind_protect
%!
%!   ## We should't need to do this, buy gzip is a bit mad and may create
%!   ## files in the current working directory.
%!   r = mkdir (testdir);
%!   cd (testdir);
%!
%!   r = mkdir (outdir);
%!   if (! r)
%!     error ("failed to create temporary directory for test");
%!   endif
%!
%!   data_file = fullfile (outdir, "test-data");
%!   outfile = [data_file ".gz"];
%!   dummy = pi;
%!   save (data_file, "dummy");
%!
%!   filelist = gzip (data_file);
%!   assert (filelist, {outfile}) # check output
%!   assert (exist (outfile), 2) # confirm file exists # FIXME
%!   assert (exist (data_file), 2) # and did not remove original file
%!
%!   ## FIXME
%!   ## gzip should dutifully re-gzip files even if they already are gz
%!   data_file = fullfile (outdir, "test-data.gz");
%!   outfile = [data_file ".gz"];
%!   dummy = pi;
%!   save (data_file, "dummy");
%!
%!   filelist = gzip (data_file);
%!   assert (filelist, {outfile}) # check output
%!   assert (exist (outfile), 2) # confirm file exists
%!   assert (exist (data_file), 2) # and did not remove original file
%!
%! unwind_protect_cleanup
%!   confirm_recursive_rmdir (false, "local");
%!   rmdir (outdir, "s");
%!   cd (owd);
%!   rmdir (testdir, "s");
%! end_unwind_protect


Carnë Draug <carandraug>
Group Member
Sat 18 Jun 2016 11:49:14 PM UTC, comment #1: 

gzip has a few other issues for Matlab compatibility which are kind of related. Honestly, I think Matlab's behaviours is buggy but we should decide what to do before "fixing" our implementation.

When FILES is a directory, we should transverse the directory to find all files, but in such case, all files that are found by transversing directories are moved to a single directory but I'm not sure which one yet:

All test with R2010b


>> ls -R
.:
bar  fdir1  fdir2  foo  lol

./fdir1:
fdir3  foo1  foo2  foo3

./fdir1/fdir3:
foo3  foo4

./fdir2:
foo6

## note that fdir1/foo3.gz got clobbered
>> l = gzip ('fdir1')
l =
    'fdir1/foo1.gz'    'fdir1/foo2.gz'    'fdir1/foo3.gz'    'fdir1/foo3.gz'    'fdir1/foo4.gz'

## note that all files got moved into fdir1, including fdir2/foo6
>> system ('find -name ''*gz'' -exec rm ''{}'' \;');
>> l = gzip ({'fdir1', 'fdir2'})
l =
    'fdir1/foo1.gz'    'fdir1/foo2.gz'    'fdir1/foo3.gz'    'fdir1/foo6.gz'    'fdir1/foo3.gz'    'fdir1/foo4.gz'

## How foo and bar are not moved because they are not found by transversing a directory
>> system ('find -name ''*gz'' -exec rm ''{}'' \;');
>> l = gzip ({'foo', 'fdir1', 'bar', 'fdir2'})
l =
    'foo.gz'    'bar.gz'    'fdir1/foo1.gz'    'fdir1/foo2.gz'    'fdir1/foo3.gz'    'fdir1/foo6.gz'    'fdir1/foo3.gz'    'fdir1/foo4.gz'

## I'm not sure how this happened but all files ended up in 'fdir1/fdir3'
>> system ('find -name ''*gz'' -exec rm ''{}'' \;');
>> l = gzip ({'fdir1', 'fdir1/fdir3/foo3', 'fdir2'})
l =

    'fdir1/fdir3/foo3.gz'    'fdir1/fdir3/foo1.gz'    'fdir1/fdir3/foo2.gz'    'fdir1/fdir3/foo3.gz'    'fdir1/fdir3/foo6.gz'    'fdir1/fdir3/foo4.gz'



Matlab, has another weird behaviour. Even when specifying normal files only, if they reside in different directories, they end up being moved around not sure where:


>> l = gzip ({'foo', 'fdir1/fdir3/foo3', 'bar', 'fdir2/foo6'})
l =
    'foo.gz'    'fdir1/fdir3/foo3.gz'    'fdir1/fdir3/bar.gz'    'fdir1/fdir3/foo6.gz'


This is a series of weird behaviours, I'd argue bugs in Matlab. Still, what behaviour do we want gzip to have in Octave?

Carnë Draug <carandraug>
Group Member
Fri 17 Oct 2014 05:42:28 PM UTC, original submission:  

Apparently this hasn't worked correctly for a very long time.  I went backwards through versions to 3.2.4 and haven't found this to ever work.

The problem is in scripts/miscellaneous/private/__xzip__.m.  I've added a note in the code:


## FIXME: This does not work when you try to compress directories
##        The resulting name is dir/.gz which is totally wrong.
movefile (cellfun (@(x) sprintf ("%s.%s", x, extension),
                   fnames, "uniformoutput", false), cwd);


The issue is the algorithm which moves files and directories to a temporary directory, compresses them in-place in the temp directory, and then moves them to the output directory.  This is pretty inefficient as an algorithm, but that is a separate issue.

The problem is that files which are compressed from say "junk" to "junk.gz" are correctly moved.  But the code attempts to create the same name for directories as for files--so "junkdir/" becomes "junkdir/.gz".

Rik <rik5>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #37978:  bug-43431-gzip-bzip2-reimplement.cset added by carandraug (35KiB - application/octet-stream)
file #37527:  xzip.cc added by carandraug (2KiB - text/x-c++src)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-24 carandraug Open/ClosedOpen Closed
    2016-07-23 jwe Open/ClosedClosed Open
    2016-07-23 carandraug StatusNone Fixed
        Open/ClosedOpen Closed
    2016-07-21 carandraug Attached File- Added bug-43431-gzip-bzip2-reimplement.cset, #37978
    2016-06-19 carandraug Attached File- Added xzip.cc, #37527
    2016-06-18 carandraug Summarygzip does not compress directories gzip does not handle directories and files in different directories

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code