Change directory for obs-studio

cg2121 commented 2 years ago

Description

If you already have a obs-studio git directory, the build script will clobber it. This changes the git directory from obs-studio to obs-studio-plugin.

Motivation and Context

Frustrated when the build script messes up my obs-studio git directory.

How Has This Been Tested?

Tested with the Linux build script, and it didn't break anything, so it probably works on macOS as well. The Windows one has not been tested yet.

Types of changes

Tweak (non-breaking change to improve existing functionality)

Checklist:

[x] My code has been run through clang-format.
[x] I have read the contributing document.
[x] My code is not on the master branch.
[ ] The code has been tested.
[x] All commit messages are properly formatted and commits squashed where appropriate.
[x] I have included updates to all appropriate documentation.

RytoEX commented 2 years ago

How, exactly, does it clobber your directory? I think I'd rather fix that than force maintaining a whole second git checkout.

glikely commented 2 years ago

Around line 55 of .github/scripts/utils.zsh/setup_obs the script does unconditional git checkout and git reset --hard. If there are any local changes in the ons-studio directory then they get unconditionally deleted.

I think if the ons-studio directory isn't clean, or is on a detached head, then the script should bail and warn the user.

PatTheMav commented 2 years ago

I agree that the build script should not mess up the checkout directory, but I'd also prefer if the build script bails out if it finds uncommitted changes instead of forcibly resetting the checkout.

PatTheMav commented 1 year ago

I thought about this once more and I've arrived at the following:

The handling of an existing OBS checkout, creating a new branch (to pin a commit), handling a "dirty" checkout and all seems overly complicated
A separate checkout has the potential to remove all that complexity (easier to maintain)
A separate checkout will use more disk space, especially because of all the nested submodules (and their histories) we'd have to fetch

I'm kinda fine with the additional disk space requirements, so this change would be right (but then we could also remove the creation of a new branch and all in the checkout itself).

Would love to hear more feedback on that.

glikely commented 1 year ago

I'm fine with a separate checkout. On my development machine it is an extra 500MB to have two trees around which is bearable. For CI the change would be a wash

Actually, the CI cost is a bigger deal for me. It would speed up the build to not need to check out obs-studio every time (even if it's a cached copy). I've been wondering about doing a version of obs-deps that adds in the obs-studio build.

On Tue, Feb 21, 2023 at 4:23 PM Patrick Heyer @.***> wrote:

I thought about this once more and I've arrived at the following:

The handling of an existing OBS checkout, creating a new branch (to pin a commit), handling a "dirty" checkout and all seems overly complicated

A separate checkout has the potential to remove all that complexity (easier to maintain)

A separate checkout will use more disk space, especially because of all the nested submodules (and their histories) we'd have to fetch

I'm kinda fine with the additional disk space requirements, so this change would be right (but then we could also remove the creation of a new branch and all in the checkout itself).

Would love to hear more feedback on that.

— Reply to this email directly, view it on GitHub https://github.com/obsproject/obs-plugintemplate/pull/45#issuecomment-1438760670, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACZSPPN44B3QTFP5EZFUTKTWYTTWJANCNFSM6AAAAAAQCH6YLU . You are receiving this because you commented.Message ID: @.***>

RytoEX commented 1 year ago

Changes look fine, concept is also ok for me - @RytoEX any opinions on this?

I feel like the better change would be to simply abort the build if the existing obs-studio dir has uncommitted changes, as mentioned here and here. Checking for uncommitted changes should be trivial. I don't see a need to do anything more complicated than that (don't bother trying to stash changes for the user and make a temp branch). Personally, I don't like maintaining multiple checkouts like this unless there is no other way to handle it.

obsproject / obs-plugintemplate