g.extension: add branch option #1130

ninsbl · 2020-11-29T22:28:09Z

Address #1128 and #625 by adding a 'branch' optionthat is only used when fetcing from online repos tht support branches...

Tests pass locally, and also e.g. this:
g.extension operation=add url=https://github.com/mundialis/t.rast.mosaic branch=main extension=t.rast.mosaic
works.

Please review.

Maybe I should add an example to the documentation? Anything else?

It is sort of a new feature, but maybe we can view lack of support for branches as a bug, after the move to git?

neteler · 2020-11-29T22:40:20Z

Thanks for this quick improvement. I wonder if "main" shouldn't be automatically accepted since the term "master" is phased out in many git projects. Say, when I tell someone to install an addon from a recently created repo outside of the GRASS Addons then once doesn't easily realize that the default branch, at least in GitHub, is now called "main".

(random link, article from Oct 2020):
https://www.zdnet.com/article/github-to-replace-master-with-main-starting-next-month/

Perhaps "main" could be tried directly if "master" isn't there?

neteler · 2020-11-29T22:42:39Z

As a sidenote, CI shows an error:

Executing <g.extension g.download.location> ...
Traceback (most recent call last):
  File "/home/runner/install/grass79/scripts/g.extension", line 2335, in <module>
    sys.exit(main())
  File "/home/runner/install/grass79/scripts/g.extension", line 2310, in main
    get_addons_paths(gg_addons_base_dir=options['prefix'])
TypeError: get_addons_paths() missing 1 required positional argument: 'branch

neteler · 2020-11-30T09:07:56Z

Thanks, now it works for both cases:

# from GRASS GIS Addons repository (default branch named "master")
g.extension extension=i.zero2null
WARNING: Extension <i.zero2null> already installed. Re-installing...
Fetching <i.zero2null> from GRASS GIS Addons repository (be patient)...
Compiling...
Installing...
Updating extensions metadata file...
Updating extension modules metadata file...
Installation of <i.zero2null> successfully finished

# from other, new GitHub repo (default branch named "main")
g.extension extension=t.rast.mosaic url=https://github.com/mundialis/t.rast.mosaic 
Fetching <t.rast.mosaic> from
<https://github.com/mundialis/t.rast.mosaic/archive/main.zip> (be
patient)...
Compiling...
Installing...
Updating extensions metadata file...
Updating extension modules metadata file...
Installation of <t.rast.mosaic> successfully finished

Could there be any other side-effects?

ninsbl · 2020-11-30T13:20:37Z

Could there be any other side-effects?

I don`t think so, as download from both external repos and the official repo work, and local extensions are covert with unit tests. Your examples above, should probably go into the test suite (probably with r.example.plus from github/gitlab instead).

In addition to adding an example with a branch to the documentation, it would probably good to extend the error message so that the users gets a hint which components of the module call to check (url, branch, or possibly proxy...)?

ninsbl · 2020-11-30T14:31:48Z

However, the change will break download from older repos and probably user scripts:

This fails now (but worked before):
g.extension r.example.plus url="https://github.com/wenzeslaus/r.example.plus"

while this works (and failed before):
g.extension r.example.plus url="https://github.com/wenzeslaus/r.example.plus" branch=master

We should probably point that out in the news following a next release? And that we make that change to follow the good reasons hosting services have for making that change...

Furthermore, we would need to coordinate a possible renaming of the main branch with a point-release and change the now hardcoded branch name accordingly...

neteler · 2020-11-30T16:13:32Z

That's why I wonder if a try-except solution could be used to try both (i.e., do not code any default answer), unless the user wants a third user defined name.

ninsbl · 2020-11-30T16:31:14Z

That's why I wonder if a try-except solution could be used to try both (i.e., do not code any default answer), unless the user wants a third user defined name.

Good idea. Had`nt thought of that. Will add a try-except for main/master.

…t exist fix install add-on e.g.: g.extension r.example.plus url='https://github.com/wenzeslaus/r.example.plus'

…the main branch doesn't exist e.g. g.extension -j

tmszi · 2020-11-30T20:21:06Z

I added try/except for main/master branch. Could @ninsbl, @neteler test it again, please?

neteler · 2020-11-30T22:24:53Z

Now all examples work!

Remaining tiny issue is that this mentions "main.zip":

g.extension r.example.plus url="https://github.com/wenzeslaus/r.example.plus"
Fetching <r.example.plus> from
<https://github.com/wenzeslaus/r.example.plus/archive/main.zip> (be
patient)...

while it really uses "master.zip". Maybe worth a final tweak around this line?

grass/scripts/g.extension/g.extension.py

Line 1497 in 16d779a

gscript.message(_("Fetching <{name}> from "

tmszi · 2020-12-01T04:51:24Z

Now all examples work!

Remaining tiny issue is that this mentions "main.zip":
g.extension r.example.plus url="https://github.com/wenzeslaus/r.example.plus"
Fetching <r.example.plus> from
<https://github.com/wenzeslaus/r.example.plus/archive/main.zip> (be
patient)...
while it really uses "master.zip". Maybe worth a final tweak around this line?

grass/scripts/g.extension/g.extension.py

Line 1497 in 16d779a

gscript.message(_("Fetching <{name}> from "

This message is printed before download add-on. Will it first try to download it from the main branch, and if such a branch doesn't exist, it will try to download it from the master branch. We can move printing this message Fetching... inside download_source_code() function.

Something like that:

GRASS natural_earth/PERMANENT:~ > g.extension r.example.plus url="https://github.com/wenzeslaus/r.example.plus"
Fetching <r.example.plus> from
<https://github.com/wenzeslaus/r.example.plus/archive/main.zip> (be
patient)...
The main branch was not found. I will try to fetch the add-on from the
master branch again.
Fetching <r.example.plus> from
<https://github.com/wenzeslaus/r.example.plus/archive/master.zip> (be
patient)...
Compiling...
Installing...
Updating extensions metadata file...
Updating extension modules metadata file...
Installation of <r.example.plus> successfully finished

Is it OK like this?

ninsbl · 2020-12-01T12:10:28Z

while it really uses "master.zip". Maybe worth a final tweak around this line?

grass/scripts/g.extension/g.extension.py

Line 1497 in 16d779a

gscript.message(_("Fetching <{name}> from "

The message would probably have to be moved to the install_extension_std_platforms function:

grass/scripts/g.extension/g.extension.py

Line 1488 in b5eefc2

def install_extension_std_platforms(name, source, url):

and maybe changed to show the URL first after successfull / failed download.

Or we add / move a URL check to that function or the download_source_code function (

grass/scripts/g.extension/g.extension.py

Line 1442 in b5eefc2

def download_source_code(source, url, name, outdev,

)?

Note also the verbose message there...

ninsbl · 2020-12-01T22:40:51Z

In the last commits I tried to consolidate the messages. Please have another look.

 g.extension r.example.plus url="https://github.com/wenzeslaus/r.example.plus"

WARNING: Extension <r.example.plus> already installed. Re-installing...
Fetching <r.example.plus> from
<https://github.com/wenzeslaus/r.example.plus/archive/main.zip> (be
patient)...
Failed with default branch. Try again from
<https://github.com/wenzeslaus/r.example.plus/archive/master.zip>...
Compiling...
Installing...
Updating extensions metadata file...
Updating extension modules metadata file...
Installation of <r.example.plus> successfully finished

scripts/g.extension/g.extension.py

Co-authored-by: Markus Neteler <neteler@gmail.com>

ninsbl · 2020-12-02T21:17:05Z

I tried to backport following the HowToGit, but got the following error message:

git cherry-pick 1ed8b2c
error: could not apply 1ed8b2c5c... g.extension: add branch option (#1130)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

Any idea?

* add branch option * use main as default branch * try download add-on from the master branch if the main branch does not exist fix install add-on e.g.: g.extension r.example.plus url='https://github.com/wenzeslaus/r.example.plus' * try download add-ons files paths json file from the master branch if the main branch doesn't exist e.g. g.extension -j * add example with branch * consolidate messages * fix a flake8 issue * use format correctly * improve message Co-authored-by: Markus Neteler <neteler@gmail.com> * improve message Co-authored-by: Markus Neteler <neteler@gmail.com> Co-authored-by: Tomas Zigo <50632337+tmszi@users.noreply.github.com> Co-authored-by: Markus Neteler <neteler@gmail.com>

neteler · 2020-12-02T21:24:29Z

Yes, the backport of #683 was missing. Done now and this one also backported.

add branch option

7ed6088

ninsbl requested review from neteler and tmszi November 29, 2020 22:28

use main as default branch

bd219fb

tmszi added 2 commits November 30, 2020 20:07

try download add-on from the master branch if the main branch does no…

16cfa7f

…t exist fix install add-on e.g.: g.extension r.example.plus url='https://github.com/wenzeslaus/r.example.plus'

try download add-ons files paths json file from the master branch if …

1def8d3

…the main branch doesn't exist e.g. g.extension -j

tmszi approved these changes Nov 30, 2020

View reviewed changes

add example with branch

2fbf51a

ninsbl added 3 commits December 1, 2020 23:13

consolidate messages

a4524a6

fix a flake8 issue

36914f0

use format correctly

a273ea0

neteler reviewed Dec 2, 2020

View reviewed changes

scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved

neteler reviewed Dec 2, 2020

View reviewed changes

scripts/g.extension/g.extension.py Outdated Show resolved Hide resolved

neteler added this to the 7.8.5 milestone Dec 2, 2020

neteler added the backport_needed label Dec 2, 2020

ninsbl and others added 2 commits December 2, 2020 18:50

improve message

f2116f9

Co-authored-by: Markus Neteler <neteler@gmail.com>

improve message

8ee1ba1

Co-authored-by: Markus Neteler <neteler@gmail.com>

neteler approved these changes Dec 2, 2020

View reviewed changes

ninsbl merged commit 1ed8b2c into OSGeo:master Dec 2, 2020

ninsbl deleted the g_extension_git_improve branch December 2, 2020 21:03

ninsbl mentioned this pull request Dec 2, 2020

[Bug] g.extension: update to also accept "main" branch in addition to "master" default branch #1128

Closed

neteler removed the backport_needed label Dec 2, 2020

ninsbl mentioned this pull request Dec 15, 2020

[Feat] g.extension: improved git support #625

Open

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

g.extension: add branch option #1130

g.extension: add branch option #1130

ninsbl commented Nov 29, 2020

neteler commented Nov 29, 2020

neteler commented Nov 29, 2020

neteler commented Nov 30, 2020

ninsbl commented Nov 30, 2020

ninsbl commented Nov 30, 2020

neteler commented Nov 30, 2020

ninsbl commented Nov 30, 2020

tmszi commented Nov 30, 2020

neteler commented Nov 30, 2020

tmszi commented Dec 1, 2020

ninsbl commented Dec 1, 2020

ninsbl commented Dec 1, 2020

ninsbl commented Dec 2, 2020

neteler commented Dec 2, 2020

g.extension: add branch option #1130

g.extension: add branch option #1130

Conversation

ninsbl commented Nov 29, 2020

neteler commented Nov 29, 2020

neteler commented Nov 29, 2020

neteler commented Nov 30, 2020

ninsbl commented Nov 30, 2020

ninsbl commented Nov 30, 2020

neteler commented Nov 30, 2020

ninsbl commented Nov 30, 2020

tmszi commented Nov 30, 2020

neteler commented Nov 30, 2020

tmszi commented Dec 1, 2020

ninsbl commented Dec 1, 2020

ninsbl commented Dec 1, 2020

ninsbl commented Dec 2, 2020

neteler commented Dec 2, 2020