xfmconcat and -clobber

[andrewjanke]

1. When run with -clobber, if the grid*.mnc files referenced in the
   xfm are symlinks, it will try to clobber the files that are referenced
   by the symlink, not the symlinks themselves, and

2. When it cannot overwrite the referent grid files (e.g., because of
   write protection or ownership), it will throw a warning, but still
   returns a 0 exit status.

So, when concat_grid_0.xfm is a symlink to a non-writeable file,
xfmconcat nominally succeeds:

$ ls -al concat_grid_0.mnc
0 lrwxrwxrwx 1 alex alex 13 2012-07-04 12:13 concat_grid_0.mnc -> /usr/bin/last*
$ xfmconcat -clobber a.xfm b.xfm concat.xfm
(from micreate): Unable to create file 'concat_grid_0.mnc'
Error: opening MINC file "concat_grid_0.mnc".
$ echo $?
0

Obviously, the resulting xfm (+grids) will not be what the user expected.

I think that at minimum, xfmconcat should fail when it can't write a
file; and I would also say that when clobbering, it should clobber a
symlink, not the referent file (but that latter point is up for debate
perhaps, which is actually why I raised it here).

[andrewjanke]

This really isn't a bug in xfmconcat but rather a bug in volume_io
somewhere around here:

https://github.com/BIC-MNI/minc/blob/master/volume_io/MNI_formats/gen_xf_io.c#L187

The question is though given that it's going to be a pain to get the
value of the clobber flag into this function what is the desired
behaviour. My thoughts would be that if the code has got to writing a
transform it should simply write the transform irrespective of whether
the target is a link or a file.

So, do we introduce a test to see if the output grid file exists
(already) and if it does, remove it? This should get around all
combinations of this that I can think of.

[andrewjanke]

I agree - as far as I can tell the simple thing to do here would be to
remove a pre-existing grid file or symlink before the new one is
written. The clobbering behaviour is (should be) already established
here (at least xfmconcat checks it right at the outset based on
existence of the output xfm).

But that still leaves the issue that the return value of output_volume
is currently ignored; if the write (or the removal) of the grid file
fails the main prog will still exit(EXIT_SUCCESS). I think
output_one_transform() should generate a meaningful return value that
is passed up through output_transform() (line 289/291).