New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch import projet from Gitlab #605

Closed
wants to merge 1 commit into
base: stretch-unstable
from

Conversation

Projects
None yet
4 participants
@Mickael-Martin
Copy link
Contributor

Mickael-Martin commented Dec 21, 2018

Please check if you are no regression. It works for me with url of project : https://framagit.org/Mickael-Martin/zabbix_ynh or url of branch : https://framagit.org/Mickael-Martin/zabbix_ynh/tree/testing

The problem

Cannot import Gitlab (an other gitlab based solutions) project branch.
...

Solution

Use correct git syntax with correct url
...

PR Status

...

How to test

...

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
Patch import projet from Gitlab
Please check if you are no regression. It works for me with url of project : https://framagit.org/Mickael-Martin/zabbix_ynh or url of branch : https://framagit.org/Mickael-Martin/zabbix_ynh/tree/testing
@Psycojoker

This comment has been minimized.

Copy link
Member

Psycojoker commented Dec 29, 2018

Hello,

I'm wondering if we should do something like :

if 'github.com/' in url: 
    # current behavior
else:
    # your patch

That would be a safer approach.

Nevertheless, thx a lot for your contribution ❤️

@Mickael-Martin

This comment has been minimized.

Copy link
Contributor

Mickael-Martin commented Jan 2, 2019

My patch is already in the "else" block, so I think I don't understand what you explain...

@@ -1901,10 +1901,10 @@ def _fetch_app_from_git(app):
# we will be able to use it. Without this option all the history
# of the submodules repo is downloaded.
subprocess.check_call([
'git', 'clone', '--depth=1', '--recursive', url,
'git', 'clone', '--recursive', url,

This comment has been minimized.

@alexAubin

alexAubin Jan 7, 2019

Member

Can you elaborate on the removal of --depth=1 here ? I don't understand how that's related to the issue :/

extracted_app_folder])
subprocess.check_call([
'git', 'reset', '--hard', branch
'git', 'reset', '--hard', 'remotes/origin/'+ branch

This comment has been minimized.

@alexAubin

alexAubin Jan 7, 2019

Member

Eh I'm surprised that just using the branch name doesn't work ? For this as well, I don't understand how that relates to the Github/Not-Github thing ? Or is it that somehow Github automatically define some sort of aliases and not other type of forges ?

This comment has been minimized.

@Mickael-Martin

Mickael-Martin Jan 9, 2019

Contributor

Actual scenario (not working)

git clone --depth=1 --recursive https://framagit.org/Mickael-Martin/zabbix_ynh
Cloning into 'zabbix_ynh'...
remote: Enumerating objects: 26, done.
remote: Counting objects: 100% (26/26), done.
remote: Compressing objects: 100% (22/22), done.
remote: Total 26 (delta 3), reused 18 (delta 0)
Unpacking objects: 100% (26/26), done.
@mike:~# cd zabbix_ynh/
@mike:~/zabbix_ynh# git branch -a -v
* master                fe30b28 Update post_user_delete
  remotes/origin/HEAD   -> origin/master
  remotes/origin/master fe30b28 Update post_user_delete
@mike:~/zabbix_ynh# git reset --hard testing
fatal: ambiguous argument 'testing': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

Solution 1 (my first proposition) :

git clone --recursive https://framagit.org/Mickael-Martin/zabbix_ynh
Cloning into 'zabbix_ynh'...
remote: Enumerating objects: 95, done.
remote: Counting objects: 100% (95/95), done.
remote: Compressing objects: 100% (47/47), done.
remote: Total 344 (delta 56), reused 82 (delta 48)
Receiving objects: 100% (344/344), 68.62 KiB | 0 bytes/s, done.
Resolving deltas: 100% (215/215), done.
@mike:~# cd zabbix_ynh/
@mike:~/zabbix_ynh# git branch -a -v
* master                 fe30b28 Update post_user_delete
  remotes/origin/HEAD    -> origin/master
  remotes/origin/master  fe30b28 Update post_user_delete
  remotes/origin/testing fe30b28 Update post_user_delete
@mike:~/zabbix_ynh# git reset --hard remotes/origin/testing
HEAD is now at fe30b28 Update post_user_delete

Solution 2, maybe more efficient

git clone -b testing --single-branch https://framagit.org/Mickael-Martin/zabbix_ynh --depth 1 --recursive
Cloning into 'zabbix_ynh'...
remote: Enumerating objects: 26, done.
remote: Counting objects: 100% (26/26), done.
remote: Compressing objects: 100% (22/22), done.
remote: Total 26 (delta 3), reused 18 (delta 0)
Unpacking objects: 100% (26/26), done.
@mike:~# cd zabbix_ynh/
@mike:~/zabbix_ynh# git branch -a -v
* testing                fe30b28 Update post_user_delete
  remotes/origin/testing fe30b28 Update post_user_delete

This comment has been minimized.

@alexAubin

alexAubin Jan 15, 2019

Member

Hmyep indeed, I can reproduce that on my computer, thanks for those details - though that's a bit unexpected / disappointing @.@ (from Git's side, or the forge side, idk)

Anyway : I think Solution 2 is indeed better and more efficient. I dunno if we have repositories such that this would matter, but I wouldnt be too surprised.

So please if you can implement the -b <branch> --single-branch that would be awesome :P (making sure that git version in stretch supports this)

@Psycojoker

This comment has been minimized.

Copy link
Member

Psycojoker commented Jan 7, 2019

My patch is already in the "else" block, so I think I don't understand what you explain...

Indeed, I've read too fast, sorry about that ^^'

@alexAubin

This comment has been minimized.

Copy link
Member

alexAubin commented Jan 16, 2019

Closing (c.f. #615 )

@alexAubin alexAubin closed this Jan 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment