octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Another package manager issue


From: David Bateman
Subject: Re: Another package manager issue
Date: Wed, 30 Aug 2006 21:44:54 +0200
User-agent: Thunderbird 1.5.0.5 (X11/20060817)

John W. Eaton wrote:
> On 29-Aug-2006, I wrote:
> 
> | On 29-Aug-2006, David Bateman wrote:
> | 
> | | John W. Eaton wrote:
> | | > On 28-Aug-2006, David Bateman wrote:
> | | >
> | | > | I see two solutions. Mark all of the
> | | > | sub-functions that call addpath in pkg.m as commands,
> | | >
> | | > I don't understand how this helps.
> | | >   
> | | Think about addpath which is a function marked as a command. When it
> | | adds a path with a PKG_ADD file in it is is evaluated correctly. I
> | | haven't looked at the code, but I suspect that if a function marked as a
> | | command is run, the level is not incremented. Therefore if all of the
> | | functions up to and including the function with the addpath are marked
> | | as commands, then addpath will appear at the top-level.
> | 
> | It's not that the addpath function is marked as a command, it is that
> | it is a built-in function.  Octave considers itself at the top level
> | if curr_sym_tab == top_level_sym_tab (see at_top_level in
> | variables.cc).  We don't reset curr_sym_tab when a built-in function
> | executes because a built-in function has no local symbol table.
> | 
> | Should built-in functions execute so that at_top_level is true?  If
> | not, we could always set curr_sym_tab to 0 while built-in functions
> | execute.
> | 
> | | > | or use evalin to
> | | > | run the addpath command in the top-level (which is what the attached
> | | > | patch does). Can this be applied
> | | >
> | | > This would also work, but would it be better to eliminate the
> | | > requirement that mark_as_command may only be called at the top level?
> | | > Or would the potential for confusion be too great?#
> | | >   
> | | Your call, though I can see that the above is a nasty syntax error whose
> | | reason is not clear. I suppose that a warning might be added for when
> | | functions are marked as commands within other functions, and I can turn
> | | this warning off in the case of the package manager, so do it however
> | | you want.
> | | 
> | | Note though that the change in the patch I sent for extract_pkgadddel
> | | sub-function is necessary as there is a variable name clash in this
> | | function that I accidentally introduced..
> | 
> | For now I think it is best to use evalin.  Will you please check in
> | your patch?
> 
> Wait, I think the change below may be a better fix.  It simply forces
> the PKG_ADD and PKG_DEL files to be sourced in the base workspace.
> No change should be needed to the pkg.m file now since executing the
> PKG_ADD and PKG_DEL files is handled internally by the addpath and
> rmpath functions.
> 
> The source_file function in parse.y now has a context arg.  This
> functionality is also available in the scripting language, so
> 
>   source ("file", "base")
> 
> is now the same as
> 
>   evalin ('source ("file")', "base")
> 
> Do you see any problems with this change?
> 
> jwe
> 
> 

Seems like a good idea and in removes the need for the evalin in pkg,
However, the second part of patch for the variable nameclash in
extract_pkgadddel needs fixing. See attached patch

D.

2006-08-26  David Bateman  <address@hidden>

        * pkg/pkg.m (create_pkgadddel): Resolve variable name-clash.
Index: scripts/pkg/pkg.m
===================================================================
RCS file: /cvs/octave/scripts/pkg/pkg.m,v
retrieving revision 1.8
diff -c -r1.8 pkg.m
*** scripts/pkg/pkg.m   26 Aug 2006 09:38:53 -0000      1.8
--- scripts/pkg/pkg.m   30 Aug 2006 19:42:51 -0000
***************
*** 551,570 ****
  function create_pkgadddel (desc, packdir, nm)
    pkg = [desc.dir "/" nm];
    fid = fopen(pkg, "wt");
    if (fid >= 0)
      ## Search all dot-m files for PKG commands
      lst = dir ([packdir "inst/*.m"]);
      for i=1:length(lst)
!       nm = lst(i).name;
!       fwrite (fid, extract_pkg (nm, ['^[#%][#%]* *' nm ': *(.*)$']));
      endfor
  
      ## Search all C++ source files for PKG commands
      lst = dir ([packdir "src/*.cc"]);
      for i=1:length(lst)
!       nm = lst(i).name;
!       fwrite (fid, extract_pkg (nm, ['^//* *' nm ': *(.*)$']));
!       fwrite (fid, extract_pkg (nm, ['^/\** *' nm ': *(.*) *\*/$']));
      endfor
  
      ## Add developer included PKG commands
--- 551,571 ----
  function create_pkgadddel (desc, packdir, nm)
    pkg = [desc.dir "/" nm];
    fid = fopen(pkg, "wt");
+ 
    if (fid >= 0)
      ## Search all dot-m files for PKG commands
      lst = dir ([packdir "inst/*.m"]);
      for i=1:length(lst)
!       nam = [packdir "inst/" lst(i).name];
!       fwrite (fid, extract_pkg (nam, ['^[#%][#%]* *' nm ': *(.*)$']));
      endfor
  
      ## Search all C++ source files for PKG commands
      lst = dir ([packdir "src/*.cc"]);
      for i=1:length(lst)
!       nam = [packdir "src/" lst(i).name];
!       fwrite (fid, extract_pkg (nam, ['^//* *' nm ': *(.*)$']));
!       fwrite (fid, extract_pkg (nam, ['^/\** *' nm ': *(.*) *\*/$']));
      endfor
  
      ## Add developer included PKG commands

reply via email to

[Prev in Thread] Current Thread [Next in Thread]