[GRASS-dev] Re: [GRASS-SVN] r49533 - grass/branches/develbranch_6/gui/scripts

2011/12/5 <svn_grass@osgeo.org>:

Author: hamish
Date: 2011-12-04 16:05:50 -0800 (Sun, 04 Dec 2011)
New Revision: 49533

Modified:
grass/branches/develbranch_6/gui/scripts/g.extension.py
Log:
avoid unnecessary build clutter in private installs (see #1501, sync with r49527);
only create build dirs for option=add

I *really* don't understand why all (almost all) the files needs to be
moved around by `g.extension`. Let's keep in GRASS6 things as they are
(how targets are defined my building system, in concrete, let's keep
man pages in `man`, don't move them to `docs\man`). Let's invest
energy to GRASS7, let's make changes directly in the building system,
in this case modify GRASS7 to install man pages to `$ARCH_DIR/docs`,
similarly for other issues. Your changes make `g.extension` in G6 more
and more complicated, less readable, please keep the things simple. I
can't see anything good when over-complicated `g.extension` tries to
"fix" standard installation made by building system. It's tricky, just
waste of time, keep to decide building system where is target of the
files. Modify build system directly, don't try to hack `g.extension`
to get what you want. GRASS7 is a playground not GRASS6.

I am asking you to revert all the recent changes in `g.extension`
(BASH and Python) and keep in GRASS6 things as defined by building
system. Then we can discuss how to change building system in GRASS7 to
satisfy all the needs.

Personally, I am confused from `g.extension` in G6, it's starting to
more and more complicated, tries to "fix" building system, RC3
requested by you. Nothing good for my POV. Please let's keep things
simple, don't introduce more and more hacks into `g.extension` in G6,
rather invest time to the changes building system in G7!

Only one acceptable "hack" for me is creating the symlink/copy for
exe/bat files ($GRASS_ADDON_PATH/bin -> $GRASS_ADDON_PATH) nothing
more.

What is the option of other developers?

Martin

Hi,

> Author: hamish
> Date: 2011-12-04 16:05:50 -0800 (Sun, 04 Dec 2011)
> New Revision: 49533
>
> Modified:
>
> grass/branches/develbranch_6/gui/scripts/g.extension.py
> Log:
> avoid unnecessary build clutter in private installs
> (see #1501, sync with r49527);
> only create build dirs for option=add

Martin:

I *really* don't understand why all (almost all) the files
needs to be moved around by `g.extension`.

here are the sum total of the grass6 addons installation:

mv bin/* .
mv scripts/* .
rmdir bin/ scripts/

mv man/ docs/

-- and that's it --

This is not some massive re-ordering, this is making
g.extension be a tidy citizen and finish its install, as it
should have been from the start.

Let's keep in GRASS6 things as they are

g.extension is a new module which so far remains ~unfinished.
6.4.2 will be the first release where it actually works for
everyone. this is our last chance to fix it before what we
decide gets locked in for the rest of the 6.x lineage.
ignoring the problem now will only make things worse.

Your changes make `g.extension` in G6 more and more
complicated, less readable, please keep the things
simple.

as above, this is basically 3 "mv" commands to finish the
install. hardly a huge amount of complication or complex
interaction.

worst case is 3 lines of string substitution to accommodate that.
If you want to go with install manifest files or whatever, I'm
happy to do write those lines of code for you too as it's not
hard and I'm willing to support it.

I can't see anything good when over-complicated `g.extension`
tries to "fix" standard installation made by building system.

For the n-th time, GRASS_ADDON_PATH is not GISBASE*. It was never
meant to be GISBASE. There is no reason for it to be exactly
matching GISBASE.

For 9 years GRASS_ADDON_PATH has been a component of the $PATH.
In the last year or so it has been hijacked to be used as
--prefix= instead. I should have nipped that in the bud when it
first happened, but to my regret I didn't.

[*] if you want to rename it GRASS_ADDON_BASE in grass7 and use
it like a fake GISBASE, I don't mind that at all, have fun.

and anyway, better to have an intelligent g.extension and a
clean user experience than a dumb g.extension and cluttered
user experience. it is one thing that Apple has figured out
since long ago: make the developers do the hard work so that
the user doesn't have to.

It's tricky, just waste of time, keep to decide building
system where is target of the files.

the shell script version has it right I think, the build system
builds the module in .tmp/, for system wide installs to GISBASE
it installs using 'make install', for installs to GRASS_ADDON_PATH
it copies the files in manually. there is really nothing highly
complicated or tricky about it.

again, the changes boil down to:
mv bin/* .
mv scripts/* .
mv man/ docs/

not exactly redefining the API, and any other module extra files
built into there remain in their natural spots.

Modify build system directly, don't try to hack `g.extension`
to get what you want. GRASS7 is a playground not GRASS6.

I find that to be a bit of a strawman argument, I would not
think for a moment to change the grass6 build system to make
g.extension 'easier'. Very much the opposite! the build system
is fine. the problem is attempting to use 'make install' when
it is not appropriate to do so.

I am asking you to revert all the recent changes in
`g.extension` (BASH and Python) and keep in GRASS6 things
as defined by building system.
Then we can discuss how to change building system
in GRASS7 to satisfy all the needs.

For the n-th + 1 time, GRASS_ADDON_PATH is not GISBASE.
The build system has nothing to do with it.

currently what is in devbr6 works, is clean, and does not
redefine/hijack GRASS_ADDON_PATH late in the stable release
series. I will not apologize for working code.

here is a sample of what 6.5svn gives you now:

$ ls addons/
d.barb* docs/ g.md5sum* i.points.auto* r.fuzzy* r.surf.volcano*

... and now that it's working you want to mess it up again? why?
for what technical gain? it works solidly now (afaik) and is nice
and clean, let's just enjoy it. If you want help with the
wingrass manifest stuff just ask, I'm happy to help.

aside:
it is a big mistake to think that the addon path only exists
for g.extension, and so can be a hidden mess of directories
which are centrally managed by software.

often it will mainly be user-managed for personal scripts.
things like two little text+execute scripts for switching back
and forth between default databases or little shortcuts like
'x0' for selecting or starting a monitor. I've got dozens of
these things before I even get to my reusable job scripts.

Personally, I am confused from `g.extension` in G6,

and I'm confused in the middle of the wxGUI code, but that's
ok and doesn't mean there is anything wrong with it, just ask..

it's starting to more and more complicated,

creating our own multi-platform CRAN-like system was never
going to be simple. but I don't forsee too many additional
changes will be needed, and am not at all temped to refactor
known working code.

tries to "fix" building system,

No, it builds naively using the build system into .tmp/; uses
the main build system for system-wide installs to GISBASE; and
for private installs to $HOME manually copies the files to where
they need to go. the build system is never molested, just used
where it is appropriate to be used, and not where it isn't.

RC3 requested by you.

it is many times better for the software and for developer-
relations :slight_smile: that we sort this out now, before release.
x1000

Only one acceptable "hack" for me is creating the symlink/copy
for exe/bat files ($GRASS_ADDON_PATH/bin -> $GRASS_ADDON_PATH)
nothing more.

a) moving executables into the real $PATH dir is not a hack.
moving the manuals into the documentation folder is not a hack.

-duplicate files, folders, and symlinks is a huge messy hack,
which doesn't have to be; it's already solved; why unsolve it?-

b) may I ask why you care where the man pages go in a private
install? or why bin/ and scripts/ must be kept separate when
nothing in the codebase cares? I'd much rather have g.extension
install to addons properly than hack the main init.sh to try to
accommodate the mess of two extra & superfluous ADDON_PATHs,
and leave the user with a needlessly cluttered home dir.

in summary-
what's in devbr6 is stable and working fine AFAIK. I asked for
testing and comments a number of times before committing with
no response. I'll ask again as I'm genuinely interested to learn
of any technical problems:

"with an empty (or non-existent) addons dir could you try [...] grass 6.5's g.extension [...] and installing a few addon modules? After seeing it I hope you agree it is a much cleaner way to organize things. If there is a technical reason not to clean that up I'd still like to hear what it is, as I know of none."

regards,
Hamish

2011/12/7 Hamish <hamish_b@yahoo.com>:

here are the sum total of the grass6 addons installation:

mv bin/* .
mv scripts/* .
rmdir bin/ scripts/

mv man/ docs/

-- and that's it --

This is not some massive re-ordering, this is making
g.extension be a tidy citizen and finish its install, as it
should have been from the start.

how you would call that, mirror reordering?:wink: Only one file which is
not touched (moved) is html file.

I simply don't understand why to waste energy with that in GRASS6.
Modify build system accordingly in GRASS7. Your changes in G65 are not
necessary, leads to more complicated `g.extension` code. I spend again
quite a lot of time with that today instead of doing *real* job.
Please move on towards GRASS7. I have nothing more to say.

Martin

--
Martin Landa <landa.martin gmail.com> * http://geo.fsv.cvut.cz/~landa

2011/12/7 Hamish <hamish_b@yahoo.com>:

RC3 requested by you.

it is many times better for the software and for developer-
relations :slight_smile: that we sort this out now, before release.
x1000

to understand - did you request RC3 for the reason to make
`g.extension` "to be tidy citizen" in G642 or for other reason? In the
case of "tidy citizen your changes were introduced only in `devbr6`. I
am afraid there is nobody how would test it (unfortunately I have been
force to do that) and it's understandable for my POV. It's just
complicates the situation before releasing 6.4.2. That's point (as
usually).

Martin

--
Martin Landa <landa.martin gmail.com> * http://geo.fsv.cvut.cz/~landa

Hi,

2011/12/7 Martin Landa <landa.martin@gmail.com>:

This is not some massive re-ordering, this is making
g.extension be a tidy citizen and finish its install, as it
should have been from the start.

how you would call that, mirror reordering?:wink: Only one file which is
not touched (moved) is html file.

seems to be like a joke - "finish its install" :wink: The fact is that
you call `make install` in `g.extension` and then you move the files
around with one exception (html file). That's bad idea itself.

My personal remarks:

1) do with `g.extension` in G6 everything what makes you satisfied. I
am not going to comment it any more.

I personally don't want to continue in this kind of discussion,
because it's not a discussion. You have decided how the things will be
at the beginning. Moreover I really feel this topic as mirror, or
better to say I am very surprised that you are able to spend with
hacking `g.extension` in G6 so much time.

2) important thing is RC3 - you have requested that, but it's not
clear (at least for me) why and what you would like to do with
`g.extension` in G64.

Martin

--
Martin Landa <landa.martin gmail.com> * http://geo.fsv.cvut.cz/~landa

Sorry to drag this on,

Martin:

I simply don't understand why to waste energy with that in GRASS6.

- to preserve design consistency with earlier grass6 addon use.
- g.extension is our newest core infrastructure module, it is just now
  becoming mature, and what we do with this now will be in use for
  a long time to come. (for however long g6 is used)
- it itches

to understand - did you request RC3 for the reason to make
`g.extension` "to be tidy citizen" in G642 or for other reason?
In the case of "tidy citizen your changes were introduced only in
`devbr6`.

I would like to backport these changes to 6.4 in time for 6.4.2.
To ensure that all is well with that a quick rc3 would be nice
once it is done. I have not backported it out of the devel branch
yet as I hoped for some testing feedback and better consensus in
this discussion -- I don't like using the stable branch for
testing, and I don't like getting into the game where the last
person to sneak in their commit before release gets their way.

Perhaps that makes this drag on way too long and eat our energy,
but I think it is still better than the alternative way of shooting
first and asking questions later.

If there is nothing more I'll do that commit tomorrow.

It's just complicates the situation before releasing 6.4.2.

better a little pain for us few now than to change it after
widespread release and cause pain for many users later on.

The fact is that you call `make install` in `g.extension` and then
you move the files around with one exception (html file).

I was mistaken when I earlier suggested that 'make install' was only
used for system-wide installs. That was used in an earlier iteration
or some uncommitted experimentation in the script, but it is not
what's in svn now. None the less, either way and whatever percentage
it is, it gives the same result of moving 3 files then removing some
directories if they are empty ...

I am very surprised that you are able to spend with hacking
`g.extension` in G6 so much time.

shrug, it's my time and this is something I'd like to see us do really
well. The new wxGUI extension tool is really nice and useful, I would
like to ensure that the backend install is the same. I don't like to
make you spend your time on things you'd rather not, so I again offer
any help you need to make this work.

You have decided how the things will be at the beginning.

Please don't think that I am so stubborn that I can never have my
mind changed in the face of solid reasoning. I am much more
concerned with finding the correct answer than personally being
always correct or getting my own way. Believe me, I would not give
you such a robust argument if I didn't respect your views or
talent and ability to show me what I hadn't considered. If after
that my ideas still haven't fallen, then I have more confidence
that they are not so silly.

2) important thing is RC3 - you have requested that, but it's not
clear (at least for me) why and what you would like to do with
`g.extension` in G64.

My plan is to backport the g.ext .sh + .py scripts that are now in
6.5svn, then if there is the will to do so put out a rc3, which if
everything works in we can release as 6.4.2 final soon after. If we
got plenty of testing in 6.4svn I guess a rc3 wouldn't be needed,
but I suspect we'd have the same people testing as who would for
6.5svn, so a rc3 might get a wider audience. shrug

(Also I would s/GRASS_ADDON_PATH/GRASS_ADDON_BASE/ in trunk)

best,
Hamish

Hi,

2011/12/11 Hamish <hamish_b@yahoo.com>:

to understand - did you request RC3 for the reason to make
`g.extension` "to be tidy citizen" in G642 or for other reason?
In the case of "tidy citizen your changes were introduced only in
`devbr6`.

I would like to backport these changes to 6.4 in time for 6.4.2.

well, please don't forget about addons on Windows. Script's are
installed to `scripts` directory and bat-files to `bin` directory. I
am not sure what tidy citizen should do in this case (because I simply
don't understand this idea), move also bash script to the main dir or
not. I will leave it on you, see [1].

[...]

Martin

[1] http://trac.osgeo.org/grass/changeset/49677

--
Martin Landa <landa.martin gmail.com> * http://geo.fsv.cvut.cz/~landa

status:

both g.ext .sh and .py merged in 6.4 and 6.5
init.sh was already sync'd (sans some whitespace)
Martin updated init.bat already

Martin wrote:

well, please don't forget about addons on Windows. Script's are
installed to `scripts` directory

see below,

and bat-files to `bin` directory.

this has changed now, right?

I am not sure what tidy citizen should do in this case
(because I simply don't understand this idea),

$GRASS_ADDONS_PATH is in the path. So user programs which need to be
in the path get put in that directory. long term survival of msys
or no, I think that everything which is in some context an executable
should end up in $PATH or %PATH%.

move also bash script to the main dir or not.

yes, I think so,

what is in the .bat file? is it just the grass-run.bat wrapper?
or is that hardcoding to scripts/$g.module somehow?

[ copied at http://paste.debian.net/149064/ ]

Index: gui/scripts/g.extension.py

--- gui/scripts/g.extension.py (revision 49698)
+++ gui/scripts/g.extension.py (working copy)
@@ -422,14 +422,13 @@
     if os.path.exists(os.path.join(options['prefix'], 'bin', options['extension']) + EXT_BIN):
         shutil.move(os.path.join(options['prefix'], 'bin', options['extension']) + EXT_BIN,
                     os.path.join(options['prefix'], options['extension']) + EXT_BIN)
+ if os.path.exists(os.path.join(options['prefix'], 'scripts', options['extension'])):
+ shutil.move(os.path.join(options['prefix'], 'scripts', options['extension']),
+ os.path.join(options['prefix'], options['extension']))
     if sys.platform == 'win32':
         if os.path.exists(os.path.join(options['prefix'], 'bin', options['extension']) + EXT_SCT):
             shutil.move(os.path.join(options['prefix'], 'bin', options['extension']) + EXT_SCT,
                         os.path.join(options['prefix'], options['extension']) + EXT_SCT)
- else:
- if os.path.exists(os.path.join(options['prefix'], 'scripts', options['extension'])):
- shutil.move(os.path.join(options['prefix'], 'scripts', options['extension']),
- os.path.join(options['prefix'], options['extension']))
     
     # move man/ into docs/
     if os.path.exists(os.path.join(options['prefix'], 'man', 'man1', options['extension'] + '.1')):

If there needs to be an install manifest I still think it would be
better to store it locally, but forgetting that for now, does this
look ok to sync grass6.xml? (generic filename: consider renaming..?)

[ copied to http://paste.debian.net/149065/ ]

Index: tools/addons/build-xml.py

--- tools/addons/build-xml.py (revision 49697)
+++ tools/addons/build-xml.py (working copy)
@@ -41,7 +41,7 @@
         fd.write('%s<keywords>%s</keywords>\n' % (' ' * indent, ','.join(keyw)))
         fd.write('%s<binary>\n' % (' ' * indent))
         indent += 4
- for f in get_module_files(m):
+ for f in get_module_files(m, g7):
             fd.write('%s<file>%s</file>\n' % (' ' * indent, f))
         indent -= 4
         fd.write('%s</binary>\n' % (' ' * indent))
@@ -53,20 +53,28 @@
             print " FAILED"
   
-def scandirs(path):
+def scandirs(path, g7 = True):
     flist = list()
     for f in glob.glob(os.path.join(path, '*') ):
         if os.path.isdir(f):
             flist += scandirs(f)
         else:
+ if not g7:
+ if f.find('bin/') == 0:
+ f.replace('bin/', '', 1)
+ elif f.find('script/') == 0:
+ f.replace('script/', '', 1)
+ elif f.find('man/') == 0:
+ f.replace('man/', 'docs/man/', 1)
+
             flist.append(f)
-
+
     return flist

-def get_module_files(name):
+def get_module_files(name, g7 = True):
     os.chdir(os.path.join(sys.argv[1], name))
     return scandirs('*')
-
+
def get_module_metadata(name):
     import grass.script.task as gtask
     path = os.environ['PATH']

what do we need to do, if anything, for building on Macs?
(ISTR there were special compiler FLAGS needed in the past,
these are still in the .sh version but were never added
to the python version. I'm not sure if they actually do
anything though; maybe for i.pr or modules that build
their own libs?)

fwiw--
AFAIK make automatically creates build+install dirs if it
needs them, no need to do that manually AFAIK. (I don't know
if that is also true for etc/ ?) or was this causing problems??

cheers,
Hamish

Hi,

2011/12/13 Hamish <hamish_b@yahoo.com>:

and bat-files to `bin` directory.

this has changed now, right?

yes

If there needs to be an install manifest I still think it would be
better to store it locally, but forgetting that for now, does this

yes

look ok to sync grass6.xml? (generic filename: consider renaming..?)

don't know `addons/grass6.xml` seems to be OK for me.

[ copied to http://paste.debian.net/149065/ ]

Index: tools/addons/build-xml.py

--- tools/addons/build-xml.py (revision 49697)
+++ tools/addons/build-xml.py (working copy)
@@ -41,7 +41,7 @@
fd.write('%s<keywords>%s</keywords>\n' % (' ' * indent, ','.join(keyw)))
fd.write('%s<binary>\n' % (' ' * indent))
indent += 4
- for f in get_module_files(m):
+ for f in get_module_files(m, g7):
fd.write('%s<file>%s</file>\n' % (' ' * indent, f))
indent -= 4
fd.write('%s</binary>\n' % (' ' * indent))
@@ -53,20 +53,28 @@
print " FAILED"

-def scandirs(path):
+def scandirs(path, g7 = True):
flist = list()
for f in glob.glob(os.path.join(path, '*') ):
if os.path.isdir(f):
flist += scandirs(f)
else:
+ if not g7:
+ if f.find('bin/') == 0:
+ f.replace('bin/', '', 1)
+ elif f.find('script/') == 0:
+ f.replace('script/', '', 1)
+ elif f.find('man/') == 0:
+ f.replace('man/', 'docs/man/', 1)
+
flist.append(f)
-
+
return flist

-def get_module_files(name):
+def get_module_files(name, g7 = True):
os.chdir(os.path.join(sys.argv[1], name))
return scandirs('*')
-
+
def get_module_metadata(name):
import grass.script.task as gtask
path = os.environ['PATH']

auto-generated metafiles on the remote server should not applied "tidy
citizen" idea. Then list of the files will be incorrect when you
install the extension with `-s` flag. g.extension should store
metafile locally in addons dir and define list of the files on it's
own.

Personal note: I hope that you understand that your idea is not eating
energy only to you... Thanks to "tidy citizen" I have always something
to do :wink:

Martin

--
Martin Landa <landa.martin gmail.com> * http://geo.fsv.cvut.cz/~landa

2011/12/13 Hamish <hamish_b@yahoo.com>:

[...]

If there needs to be an install manifest I still think it would be
better to store it locally, but forgetting that for now, does this

done in r49716.

Martin

--
Martin Landa <landa.martin gmail.com> * http://geo.fsv.cvut.cz/~landa