[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