Check git status on upstream `ultralytics` or `origin` dynamically #8694

pourmand1376 · 2022-07-23T20:14:24Z

This repo has 10k forks. Mosts of them would never update their repos and use outdated versions. We should warn them since this has great cost. Currently, It just compares it to master. This approach doesn't work for forked repos!

git remote -v

origin	https://github.com/pourmand1376/yolov5 (fetch)
origin	https://github.com/pourmand1376/yolov5 (push)
ultralytics	https://github.com/ultralytics/yolov5.git (fetch)
ultralytics	https://github.com/ultralytics/yolov5.git (push)

Then, I separate them by '\n' character and find the name of the remote which has the address of main repo. For normal repos, this is origin, but for forked repos, this doesn't exist! We should create it for the first time and use it afterwards.

This is because I have wasted two days of my time debugging and finding errors in the code that has been already fixed. I think we should really warn users for outdated repos.

🛠️ PR Summary

_{Made with ❤️ by Ultralytics Actions}

🌟 Summary

Enhanced git status checks in YOLOv5 with more robust handling of git remotes.

📊 Key Changes

🔄 The check_git_status function can now specify a repository.
🔍 Improved detection of the repository's remote URL.
🔄 If the necessary remote does not exist, it is added dynamically.
🗂️ The git fetch command now uses the correct remote.
🆕 Added a more dynamic way to determine the number of commits behind the upstream.

🎯 Purpose & Impact

🤝 This update makes the codebase more flexible by allowing status checks against different repositories.
🔗 By automatically adding missing remotes, users are eased from manual git configuration steps.
🛠 The dynamic commit comparison reduces the likelihood of errors when checking if the local repository is up to date.
🔄 Potential impact: Easier maintenance for users with forks or customized setups of the YOLOv5 repository.

pourmand1376 · 2022-07-24T04:37:58Z

Is this idea interesting to you?
@glenn-jocher

Mandroide · 2022-07-24T14:26:48Z

I

This repo has 10k forks. Mosts of them would never update their repos and use outdated versions. We should warn them since this has great cost. Currently, It just compares it to master. This approach doesn't work for forked repos!
git remote -v
origin	https://github.com/pourmand1376/yolov5 (fetch)
origin	https://github.com/pourmand1376/yolov5 (push)
ultralytics	https://github.com/ultralytics/yolov5.git (fetch)
ultralytics	https://github.com/ultralytics/yolov5.git (push)
Then, I separate them by '\n' character and find the name of the remote which has the address of main repo. For normal repos, this is origin, but for forked repos, this doesn't exist! We should create it for the first time and use it afterwards.

This is because I have wasted two days of my time debugging and finding errors in the code that has been already fixed. I think we should really warn users for outdated repos.

I find two commits noisy for the history of the repo:

[pre-commit.ci] auto fixes from pre-commit.com hooks
[Merge branch 'add_update_branch' of https://github.com/pourmand1376/yolov5

It would be good if you rewrite history and cut them back to just these two commits:
add check
use else clause

pourmand1376 · 2022-07-25T09:44:47Z

@Mandroide
This can be easily done using squash and merge option in GitHub when merging in finalized. See here.

However I am willing to fix it if you have disabled squash commits in GitHub.

pourmand1376 · 2022-07-25T11:55:23Z

This is now squashed into one commit!

glenn-jocher · 2022-07-25T11:56:53Z

I
This repo has 10k forks. Mosts of them would never update their repos and use outdated versions. We should warn them since this has great cost. Currently, It just compares it to master. This approach doesn't work for forked repos!
git remote -v
origin	https://github.com/pourmand1376/yolov5 (fetch)
origin	https://github.com/pourmand1376/yolov5 (push)
ultralytics	https://github.com/ultralytics/yolov5.git (fetch)
ultralytics	https://github.com/ultralytics/yolov5.git (push)
Then, I separate them by '\n' character and find the name of the remote which has the address of main repo. For normal repos, this is origin, but for forked repos, this doesn't exist! We should create it for the first time and use it afterwards.
This is because I have wasted two days of my time debugging and finding errors in the code that has been already fixed. I think we should really warn users for outdated repos.
I find two commits noisy for the history of the repo:

[pre-commit.ci] auto fixes from pre-commit.com hooks [Merge branch 'add_update_branch' of https://github.com/pourmand1376/yolov5

It would be good if you rewrite history and cut them back to just these two commits: add check use else clause

Yes don't worry, only Squash and merge commits are allowed for PRs so all the individual commits get squashed into just one.

glenn-jocher · 2022-07-25T12:07:16Z

@pourmand1376 this PR is a very good idea, but I wonder if there's a more concise way to implement this.

The steps are 1) determine if ultralytics/yolov5:master is added as a remote origin, and 2) if not add it, then 3) get the diff using the existing code.

pourmand1376 · 2022-07-28T09:47:46Z

@glenn-jocher
Actually I haven't found any easier way to solve this. We have git ls-remote but that's totally for another purpose.

For checking existence of a remote, I first do a git remote -v then analyse lines to see if that exists and if that doesn't exist, I add it.

democat3457 · 2022-07-28T19:17:51Z

This will not work if ssh is used as the cloning method. Perhaps the url search should be changed to just ultralytics/yolov5?

glenn-jocher · 2022-07-29T13:11:56Z

@pourmand1376 would @democat3457's idea here make this simpler?

This will not work if ssh is used as the cloning method. Perhaps the url search should be changed to just ultralytics/yolov5?

pourmand1376 · 2022-07-31T01:18:06Z

I agree with @democat3457 but that would only remove https:// part in that string.

Other that this, I think git should have a one-liner which outputs remote's name based on url. I haven't found that yet. Does anybody know any commands for this purpose?

If that's not the case, I think I can rewrite the whole thing using list comprehension to make it simpler. I will do it as soon as I have my laptop :)

glenn-jocher · 2022-07-31T15:07:25Z

@pourmand1376 I've made a few updates. The current implementation will not create new remotes if ultralytics/yolov5 is already linked to origin, and it should also be compatible with SSH clones (original PR only works for HTTPS).

glenn-jocher · 2022-07-31T15:10:12Z

This also now advises on proper git pull ultralytics master command in case remote ultralytics needs to be added.

glenn-jocher · 2022-07-31T15:14:26Z

@pourmand1376 can you test this on your side?

glenn-jocher · 2022-07-31T16:17:57Z

@pourmand1376 PR is merged. Tested locally with a fork and everything works correctly.

Thank you for your contributions to YOLOv5 🚀 and Vision AI ⭐

glenn-jocher · 2022-07-31T16:19:41Z

@AyushExel this should make it easier for forks to stay up to date. Automatically adds a new ultralytics remote and fetches from there for comparison to indicate how many commits behind your fork is. Also indicates correct git pull ultralytics master command to user to update fork.

pourmand1376 · 2022-07-31T21:44:19Z

Thanks for all of this!

I also tested locally and It works just fine.

But, When I tested on colab, it says that:

github: skipping check (Docker image), for updates see https://github.com/ultralytics/yolov5

I do not think this should be the case. Colab users should also get their updates. However I think I should report this bug on another issue.

glenn-jocher · 2022-08-01T00:16:41Z

@pourmand1376 ah thanks for spotting the Colab issue, I see it too. This is likely related to a recent update to is_docker() in #8711

glenn-jocher · 2022-08-01T01:03:43Z

@pourmand1376 Colab issue should be resolved now in #8813

Not sure if I've introduced a Docker issue now though. Waiting for the Docker image to update and will test.

EDIT: All Colab and Docker issues resolved.

AyushExel · 2022-08-01T04:36:59Z

This is a great feature!
@glenn-jocher maybe update the CONTRIBUTING.md "keeping up to date with master branch" section with this command?

glenn-jocher · 2022-08-01T10:18:47Z

@AyushExel yeah this is great. It's too bad the current 10k forks don't have this, but at least the next 10k will!

glenn-jocher · 2022-09-04T12:00:32Z

@pourmand1376 got a slight issue with this. Was working on my local fork and when I pushed to the fork it also pushed to ultralytics repo by accident since it had been added for both fetching and pulling.

Most users won't have this problem as they don't have push permissions to ultralytics/yolov5 but a few employees do so we need to be very careful.

I couldn't find any easy way to add a remote only for fetching but not pushing, i.e. https://stackoverflow.com/questions/7556155/git-set-up-a-fetch-only-remote

So I thought what if we simply do not add a remote but instead explicitly git fetch directly from ultralytics master, i.e.:
https://stackoverflow.com/questions/7556155/git-set-up-a-fetch-only-remote

$ git fetch https://github.com/ultralytics/yolov5
  
From https://github.com/ultralytics/yolov5
 * branch            HEAD       -> FETCH_HEAD

pourmand1376 · 2022-09-04T13:36:35Z

One idea is to add a .env file for developers and add a parameter in there. If the file exists and the parameter is set to True, this function should be disabled altogether.

Also, I think having a .env file for developers is a pretty common way.

Note that this file should not be committed to repo. This one should be in .gitignore and this will also be useful for extra parameters when in need.

glenn-jocher · 2022-09-06T15:59:17Z

@pourmand1376 are there any downsides to simply fetching directly from the master URL?

git fetch https://github.com/ultralytics/yolov5

…ltralytics#8694) * Add remote ultralytics and check git status with that * Simplify * Update general.py * Update general.py * s fix Co-authored-by: Glenn Jocher <glenn.jocher@ultralytics.com>

pourmand1376 · 2022-09-09T06:14:32Z

@glenn-jocher
Do you mean we should fetch from the master URL without adding the remote?

glenn-jocher · 2022-09-09T10:07:46Z

@pourmand1376 yes exactly. Then there's no push attempts to ultralytics master, i.e. we wouldn't need to add it as a remote

pourmand1376 marked this pull request as ready for review July 23, 2022 20:18

Add remote ultralytics and check git status with that

d6f20c4

pourmand1376 force-pushed the add_update_branch branch from ba4a9dc to d6f20c4 Compare July 25, 2022 11:54

Merge branch 'master' into add_update_branch

4b9c663

pourmand1376 added 2 commits July 27, 2022 02:22

Merge branch 'master' into add_update_branch

0a3c0fb

Merge branch 'master' into add_update_branch

b24a6a0

glenn-jocher and others added 3 commits July 29, 2022 15:11

Merge branch 'master' into add_update_branch

1d51a37

Merge branch 'master' into add_update_branch

894b475

Merge branch 'master' into add_update_branch

3394f02

Simplify

9e7ec7d

glenn-jocher added the TODO label Jul 31, 2022

glenn-jocher added 3 commits July 31, 2022 16:13

Merge branch 'master' into add_update_branch

f132cbe

Update general.py

b5f4bd1

Update general.py

90bde61

s fix

a1d8efd

glenn-jocher changed the title ~~Add remote ultralytics and check git status with that~~ Check git status on upstream ultralytics or origin dynamically Jul 31, 2022

glenn-jocher assigned pourmand1376 Jul 31, 2022

glenn-jocher merged commit afa403a into ultralytics:master Jul 31, 2022

glenn-jocher removed the TODO label Jul 31, 2022

pourmand1376 deleted the add_update_branch branch July 31, 2022 21:46

glenn-jocher mentioned this pull request Aug 1, 2022

fix: broken is_docker check #8711

Merged

Hojland mentioned this pull request Oct 17, 2022

feat/bump Go-Autonomous/yolov5#15

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check git status on upstream `ultralytics` or `origin` dynamically #8694

Check git status on upstream `ultralytics` or `origin` dynamically #8694

pourmand1376 commented Jul 23, 2022 •

edited by UltralyticsAssistant

Loading

pourmand1376 commented Jul 24, 2022

Mandroide commented Jul 24, 2022

pourmand1376 commented Jul 25, 2022 •

edited

Loading

pourmand1376 commented Jul 25, 2022

glenn-jocher commented Jul 25, 2022

glenn-jocher commented Jul 25, 2022 •

edited

Loading

pourmand1376 commented Jul 28, 2022

democat3457 commented Jul 28, 2022

glenn-jocher commented Jul 29, 2022

pourmand1376 commented Jul 31, 2022 •

edited

Loading

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

pourmand1376 commented Jul 31, 2022 •

edited

Loading

glenn-jocher commented Aug 1, 2022

glenn-jocher commented Aug 1, 2022 •

edited

Loading

AyushExel commented Aug 1, 2022

glenn-jocher commented Aug 1, 2022

glenn-jocher commented Sep 4, 2022 •

edited

Loading

pourmand1376 commented Sep 4, 2022

glenn-jocher commented Sep 6, 2022

pourmand1376 commented Sep 9, 2022 •

edited

Loading

glenn-jocher commented Sep 9, 2022 •

edited

Loading

Check git status on upstream ultralytics or origin dynamically #8694

Check git status on upstream ultralytics or origin dynamically #8694

Conversation

pourmand1376 commented Jul 23, 2022 • edited by UltralyticsAssistant Loading

🛠️ PR Summary

🌟 Summary

📊 Key Changes

🎯 Purpose & Impact

pourmand1376 commented Jul 24, 2022

Mandroide commented Jul 24, 2022

pourmand1376 commented Jul 25, 2022 • edited Loading

pourmand1376 commented Jul 25, 2022

glenn-jocher commented Jul 25, 2022

glenn-jocher commented Jul 25, 2022 • edited Loading

pourmand1376 commented Jul 28, 2022

democat3457 commented Jul 28, 2022

glenn-jocher commented Jul 29, 2022

pourmand1376 commented Jul 31, 2022 • edited Loading

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

glenn-jocher commented Jul 31, 2022

pourmand1376 commented Jul 31, 2022 • edited Loading

glenn-jocher commented Aug 1, 2022

glenn-jocher commented Aug 1, 2022 • edited Loading

AyushExel commented Aug 1, 2022

glenn-jocher commented Aug 1, 2022

glenn-jocher commented Sep 4, 2022 • edited Loading

pourmand1376 commented Sep 4, 2022

glenn-jocher commented Sep 6, 2022

pourmand1376 commented Sep 9, 2022 • edited Loading

glenn-jocher commented Sep 9, 2022 • edited Loading

Check git status on upstream `ultralytics` or `origin` dynamically #8694

Check git status on upstream `ultralytics` or `origin` dynamically #8694

pourmand1376 commented Jul 23, 2022 •

edited by UltralyticsAssistant

Loading

pourmand1376 commented Jul 25, 2022 •

edited

Loading

glenn-jocher commented Jul 25, 2022 •

edited

Loading

pourmand1376 commented Jul 31, 2022 •

edited

Loading

pourmand1376 commented Jul 31, 2022 •

edited

Loading

glenn-jocher commented Aug 1, 2022 •

edited

Loading

glenn-jocher commented Sep 4, 2022 •

edited

Loading

pourmand1376 commented Sep 9, 2022 •

edited

Loading

glenn-jocher commented Sep 9, 2022 •

edited

Loading