ronivay / XenOrchestraInstallerUpdater

Xen Orchestra install/update script
GNU General Public License v3.0
1.16k stars 189 forks source link

changing repo url in cfg file does not change repo pointer in git xo-src folder #128

Closed ITJamie closed 2 years ago

ITJamie commented 2 years ago

changing repo url in cfg file does not change repo pointer in git xo-src folder. This can be useful when changing between upstream git and forked repos

some ideas to fix this.

Ive also noticed that if you do delete the xo-src folder it will clone the repo back, but it will not check out the requested branch from the config file. on the next install/upgrade it will change the branch

ronivay commented 2 years ago

Hi,

Thanks for report/suggestions. First one should probably just recreate the xo-src directory, dealing with possible conflicts by just changing remote sounds something that's not worth the implementation.

Latter is definitely a bug and needs to be fixed.

Will try to check these soon.

ronivay commented 2 years ago

Hi,

I can't reproduce latter. Could you double check and let me know how it can be reproduced. Note that branch in xo-src directory doesn't change and that's by design, new installation/update is copied from that and branch is checked out in the actual build. Looking at the code and how my tests go, branch is checked out correctly no matter if xo-src directory exists or not.

Working on a fix for the changed remote.

ronivay commented 2 years ago

@ITJamie could you check my comment above, thanks.

sboyd-m commented 2 years ago

Regarding the main issue here, which we just ran across, the correct behavior would be to use git commands that already exist for this purpose.

Check remote: git remote get-url origin

If its different in the config, change it git remote set-url origin <config url>

Pull it down git fetch --prune origin

Hard switch to config branch git reset --hard origin/<branch> git clean -xdff

Deleting and recloning is kind of "I don't understand what's happening, so I'm just gonna blow it away". I think the script should be a bit more intentional than that.

sboyd-m commented 2 years ago

Acutally even better, this should always be done. Because force pushes will break named branches. Don't bother checking the remote first.

ronivay commented 2 years ago

Deleting and recloning is kind of "I don't understand what's happening, so I'm just gonna blow it away". I think the script should be a bit more intentional than that.

Yeah i was going to check if remote has changed and then delete/clone from scratch. End result would've not been much different, just more simpler.

Acutally even better, this should always be done. Because force pushes will break named branches. Don't bother checking the remote first.

This is a good point and makes above suggestion more appealing. Let's work on this on PR.

sboyd-m commented 2 years ago

@ronivay I have a PR open 🙂

ronivay commented 2 years ago

@ronivay I have a PR open 🙂

I know, was referring to that PR :) commented there couple days ago.

sboyd-m commented 2 years ago

I do not see anything?

Screen Shot 2022-06-27 at 11 49 11 AM

ronivay commented 2 years ago

I do not see anything?

My bad, review was left pending.