[Bug: 21.4.15] current directory in subprocesses is one level up

[felix.g at tzemeier.info]

Hi Vin and XEmacs-beta,

>>>>> On Tue, 10 Aug 2004 20:44:08 -0400, Vin Shelton <acs at xemacs.org> said:

    Vin> "Felix H. Gatzemeier" <f.g at tzemeier.info> writes:

    Felix> ================================================================
    Felix> Dear Bug Team!
    Felix> 
    Felix> I have something here that _is_ fixed in the current beta,
    Felix> but I'd like to suggest a different solution.
    Felix> 
    Felix> When trying to mc-sign messages in the current XEmacs
    Felix> stable release, I only got a 'wrong type argument:
    Felix> (stringp, nil)'.  Researching this, I found strange code in
    Felix> old-call-process-internal.  The buffer's current directory
    Felix> is normalized in order to serve as the current directory
    Felix> for the current process.  But this normalization
    Felix> 
    Felix> (a) does not expand "~" constructs and
    Felix> 
    Felix> (b) goes one level up in the file system hierarchy, if the
    Felix> file name does not end in a slash.
    Felix> 
    Felix> In the current beta, this is solved by tacking on a slash,
    Felix> and directory does seem to be protected from being set to
    Felix> "~".

I have to correct myself on this a bit: In the current beta,
Fexpand_file_name is also called (process.c/Fstart_process_internal
calls fileio.c/expand_and_dir_to_file, which calls
Fexpand_file_name).  So, current_buffer->directory does not have to be
"~"-clean.

    Felix> I fixed this by
    Felix> 
    Felix> (a) calling Fexpand_file_name and
    Felix> 
    Felix> (b) Introducing a function unhandled-directory-name that
    Felix> works like unhandled-file-name-directory, but does not call
    Felix> file-name-directory.
    Felix> 
    Felix> fgrep'ing through the sources, I also found
    Felix> unhandled-file-name-directory being used in
    Felix> start-process-internal (where I applied analogous changes)
    Felix> and sys_subshell (which I did not change since I had no
    Felix> idea of how to test my changes.  But that code looks *very*
    Felix> strange indeed).
    Felix> 
    Felix> My fix can still be applied in the current beta, where it
    Felix> may increase robustness and make the code more
    Felix> understandable.  Other readers like me will probably not
    Felix> understand why the current directory should be rewritten
    Felix> with a trailing slash so a function that is supposed to
    Felix> tell the directory a file resides in is tricked into not
    Felix> doing its job.
    Felix> 
    Felix> Also, the pair of current_buffer->directory and
    Felix> default-directory seems to be less synchronized now.  It
    Felix> would be nice if the documentation (read: the declaration
    Felix> of directory) had anything to say about their relationship.
    Felix> 
    Felix> 
    Felix> I have attached a patch containing my changes.

These seem to have been lost now, so I re-attached them.  They include
some other bits of added documentation.

    Felix> These are my first changes in Emacs C code, so some
    Felix> scrutiny is still recommended.  For one thing, I don't have
    Felix> any idea of how garbage collection may shoot my code in the
    Felix> foot.

    Vin> Felix,

    Vin> I think I would favor an approach that does: (a) calls
    Vin> Fexpand_file_name and (b) tacks a slash on the end of the
    Vin> directory name.

    Vin> This seems like a less invasive approach for the stable
    Vin> branch.  Do you want to try to cons that up?

Personally, I'd prefer the unhandled-directory-name solution, because
it is understandable without deep knowledge about the implementation
of unhandled-file-name-directory.  But this slash-on, slash-off seems
to be so common in the code that only newcomers find it disturbing.

I also don't see my change as much more intrusive:  It affects three
functions (Fstart_process_internal, start-process-internal, and
sys_subshell) that would have to be changed for every fix.  Beyond
that, it only introduces a small function with an identifiable
purpose.

    Vin> Does anyone else have an opinion?

Especially Ben, who seems to have working on this for quite some time?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: fxg-process.diff
Type: application/unified-diff
Size: 5872 bytes
Desc: Diffs of my changes relative to 21.4.15
Url : http://calypso.tux.org/pipermail/xemacs-beta/attachments/20040813/e276086f/attachment.bin 
-------------- next part --------------

Best wishes,

-- 
Felix Heinrich Gatzemeier                        felix.g at tzemeier.info
Phone: +49 (241) 4 01 69 28
Disclaimer: I do not speak for anyone but myself.
Please do not send me mails containing documents in proprietary
formats (such as Microsoft Word) unless you really need to.