bug#927: vc-bzr.el with cygwin bzr

bug#927: vc-bzr.el with cygwin bzr

[Phillip Lord]

Problem: vc-bzr.el doesn't work with cygwin bzr

I've recently been trying out bzr on windows and was pleased to find
vc-bzr.el. It works well with the windows bzr executable but,
unfortunately, this doesn't deal with symlinks which is important to me.
So I tried cygwin instead. Unfortunately vc-bzr.el doesn't work with
this failing with "No such file or program" errors. vc-svn and vc-cvs
work fine with cygin. 


Cause:

bzr is a python file. On cygwin it uses a magic #!/usr/bin/python line,
which vc "start-process" doesn't work with. 

I tested this with following hack, which launches python and gives the
location of bzr (under cygwin as it's cygwin python) as an argument. 

(defun vc-bzr-command (bzr-command buffer okstatus file-or-list &rest args)
  "Wrapper round `vc-do-command' using `vc-bzr-program' as COMMAND.
Invoke the bzr command adding `BZR_PROGRESS_BAR=none' and
`LC_MESSAGES=C' to the environment."
  (let ((process-environment
         (list* "BZR_PROGRESS_BAR=none" ; Suppress progress output (bzr >=0.9)
                "LC_MESSAGES=C"         ; Force English output
                process-environment)))

    (apply 'vc-do-command (or buffer "*vc*") okstatus "python"
           file-or-list "/usr/bin/bzr" bzr-command args)))

vc-bzr now works. 


Suggested Solution:

My hack is platform specific. A better solution would be, to modify
vc-bzr-command to be either of the form "bzr-program-name" or '("python
name" "bzr name"). vc-bzr-command would need to be modified to cope.
There is couple of other places vc-bzr-command is used which would need
changing also. 


Thanks for your attention!

Phil 

bug#927: vc-bzr.el with cygwin bzr

[Stefan Monnier]

> My hack is platform specific. A better solution would be, to modify
> vc-bzr-command to be either of the form "bzr-program-name" or '("python
> name" "bzr name"). vc-bzr-command would need to be modified to cope.
> There is couple of other places vc-bzr-command is used which would need
> changing also. 

A better solution would be to write a w32 wrapper for Bzr (an plain w32
executable that runs python with the bzr script), so that it does not
rely on cygwin's own handling of #!

After all, does Cygwin's bzr work with any other program that's not part
of Cygwin?  I'd guess not, which is why I think the problem is not
specific to Emacs.


        Stefan

bug#927: vc-bzr.el with cygwin bzr

[Phillip Lord]

>>>>> "Stefan" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

  >> My hack is platform specific. A better solution would be, to modify
  >> vc-bzr-command to be either of the form "bzr-program-name" or
  >> '("python name" "bzr name"). vc-bzr-command would need to be
  >> modified to cope. There is couple of other places vc-bzr-command is
  >> used which would need changing also.

  Stefan> A better solution would be to write a w32 wrapper for Bzr (an
  Stefan> plain w32 executable that runs python with the bzr script), so
  Stefan> that it does not rely on cygwin's own handling of #!

I don't think that the two contradict. Both would be possible. But, yes,
a bzr.bat in cygwin would probably solve the problem. 

  Stefan> After all, does Cygwin's bzr work with any other program
  Stefan> that's not part of Cygwin? I'd guess not, which is why I think
  Stefan> the problem is not specific to Emacs.

This depends on how they launch bzr; for vc-bzr, for instance, if vc
used an external shell-command instead of start-process it would work.
Even if emacs was using dos as it's shell, I could reconfigure
bzr-command to be "c:/cygwin/bin/python bzr"; unfortunately, you can't
do this with start-process because the space is interpreted as part of
the command name, and bzr not considered an argument. 

Given that the change I suggested is quite small, is there a problem
with putting it in; I'm willing to send in a patch if you wish. It would
also support the use case where one the user wishes to use a specific
python to run bzr. It should be transparent to other users. 

In the meantime, if I can work out how to do it, I'll write to the
cygwin packager and ask for a bzr.bat to be added to cygwin (having
tested that it works). As you say, it would help to make it more
usuable, irrespective of emacs. 

Phil

bug#927: vc-bzr.el with cygwin bzr

[Stefan Monnier]

> Given that the change I suggested is quite small, is there a problem
> with putting it in;

Yes, because I think it's just a workaround whereas a real bug fix is
easy to do.  Don't use a hack when you can use a clean solution.


        Stefan

bug#927: vc-bzr.el with cygwin bzr

[Phillip Lord]

>>>>> "Stefan" == Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  >> Given that the change I suggested is quite small, is there a
  >> problem with putting it in;

  Stefan> Yes, because I think it's just a workaround whereas a real bug
  Stefan> fix is easy to do. Don't use a hack when you can use a clean
  Stefan> solution.

Stefan

I'm not 100% I agree that this is a hack -- enabling people to specify
a python interpreter is useful outside of this difficulty. 

Still, the batch file solution also works; as you say, it's works for
other things also (DVC.el for instance!). I'll see whether I can get
this into the cygwin package so that it just does the right thing. 

Thanks for your help!

Phil

bug#927: vc-bzr.el with cygwin bzr

[Phillip Lord]

>>>>> "Stefan" == Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

  >> Given that the change I suggested is quite small, is there a problem with
  >> putting it in;

  Stefan> Yes, because I think it's just a workaround whereas a real bug fix
  Stefan> is easy to do. Don't use a hack when you can use a clean solution.

Stefan

I tried cygwin, but the answer seems to be no. 

I understand why you don't want to amend vc-bzr; would it at least be possible
to put a comment into "known bugs" or commentary, saying that it works with
cygwin but needs a wrapper script? This would at least save people wishing to
do this from having to hunt through the code? 

Thanks for your time; I'll stop mailing about this now. 

Phil