Skip to content
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

Integrate changes from dsmithni/LabViewGitEnv with sourcetree support #2

Open
joerg opened this issue May 6, 2014 · 5 comments
Open

Comments

@joerg
Copy link
Owner

joerg commented May 6, 2014

@dsmithni [1] did some great work to make this project run with sourctree [2]. This issue is intended to integrate his work into this project to make it even more awesome. But first things first.

What needs to work

On a simple git repo these commands need to work in command line and via sourcetree:

  • git diff file.vi
  • git checkout master && git merge my_branch

If they work everything else should work too. We could create a sample repo with two branches, some commits and simple VIs to test this.

What needs to be done

  • Testing if everything works with sourcetree and via command line.
  • Refactoring some code, espacially in LVCompareWrapper.sh.
    • Most sed's can be combined in two functions: One doing FWSPATH -> FIXSYMUSR -> FIXSYMTMP, and one doing REMLEADDOT -> PATHFIX -> TRAILFIX
    • Resolving of relative path in LVCompareWrapper.sh should be a function.
    • Create a function for "make path to windows"

What could be done

I never liked using a "windows" branch too much. Maybe we should move everything into one branch and create kind of an installer, or an installable zip. A ZIP would be nice, we it had to be hosted somewhere and it would be completely static. An installer script would be nice and could do some of the configuration work, but we would have to write it.

We really need to test merging capabilities of LabView. Maybe a good solution would be to merge VIs and create a simple merge commit and later on fix the VIs. I just don't think that bigger projects can be merged in a nice way.

[1] https://github.com/dsmithni/LabViewGitEnv
[2] http://www.sourcetreeapp.com/

@smithed
Copy link

smithed commented May 7, 2014

Hey Joerg,

I like your list of to-dos. I saw an example of a shell function in your script and it looks pretty straightforward. I thought about that as I was writing, but since I have such a tenuous grasp on the language, baby steps on the data made it easy to understand :)

In the commit I made earlier, I changed /tmp/ to use the mingw $HOME variable, so its no longer linked to me--I'm not sure if the way I did it is the easiest, but it works. I also tried to comment what I added, but since I was learning as I played with the script, there are probably places where I modified things unnecessarily because I was just doing it wrong. Oh well.

That having been said, I had already done what you described above and put together a test repo.
Sourctree works with the following:
Diff curr, prev
Diff prev1, prev2
Merge, with the restriction that it assumes the merge is always successful. I have no idea how to create any sort of dialog, since its called by the gui. The right answer may be to make it always fail when used in gui mode, and then people can just tell git to accept A.vi, where A.vi is actually the copy you just merged.

Command line works with the following:
git difftool -t sourcetree (or git difftool -t lvdifftool)
git mergetool -t sourcetree (or lvmergetool)

Command line also works with:
git diff
git merge
...with the restriction being that using the attributes file means sourcetree stops working. I added the flag -noa to at least conditionally disable the attributes, but I have no idea why this is happening and sourcetree doesn't offer any insight into what calls it is making. This is the biggest hurdle to overcome. I managed to get part of the way to a driver string which let diff work, but merge...I managed to make git pass a set of empty parameters to the script. That's about as far as I got :(

Anyway, as I mentioned in the email I've got a series of things going on for the next week and a half, so I probably won't have more time to work on it until at least the week of the 19th...but I'm sure it can wait a week. Thanks for your interest!

@smithed
Copy link

smithed commented May 16, 2014

"Refactoring some code, espacially in LVCompareWrapper.sh.
Most sed's can be combined in two functions: One doing FWSPATH -> FIXSYMUSR -> FIXSYMTMP, and one doing REMLEADDOT -> PATHFIX -> TRAILFIX
Resolving of relative path in LVCompareWrapper.sh should be a function.
Create a function for "make path to windows""

Done

"Testing if everything works with sourcetree and via command line."
75%

Looks like diff was an issue because of relative path, so I changed the init script to give a full path to the sh file. Still having issues with merge. No parameters are passed to the script by git, as is. I took a look at the git config --system -l results after calling LVInit and the parameters are not being added to the configuration--just the script path. Not sure whats going wrong.

Would you mind taking a look at the init script?

@joerg
Copy link
Owner Author

joerg commented May 16, 2014

Wow, there really is something going on. I didn't have much time yet looking into it but there is one thing a found, maybe that hepls.
In line 43 and 44 of LVInit you use ${PWD} which is not defined. Maybe there is a general misunderstanding:

$PWD ...This gives the value of the Variable PWD
${PWD} ...More explicit use of PWD. It is always best to use this explicit notation.
'${PWD}' ...Notice the single quotes. This is the String "${PWD}" (without the quotes)
"${PWD}" ...Notice the double quotes. This is the value of PWD represented as string (although its bash, so data types are more like meh)
$(pwd) ...Notice the round bracket. This is the value returned by executing the command pwd.

So line 44 should look like this: DIFFCMD="$(pwd)/bin/LVCompareWrapper.sh"

Also, you did remove the explicit Variable use with the curly bracktes several times. Please add the brackets again. I know they are not needed always, but they can save hours of bebugging time. They are pretty much life guards... ;)

Also in LVConfig you used:
TEMP=$RELBASE"/"$tPath
I've never seen this syntax before but it seems to work. Anyways, I think using an explicit syntax would be better and a lot easier to read:
TEMP="${RELBASE}/${tPath}"

I will have a closer look into it once I am at a Windows PC again at University on monday. Great work, keep it up... thumbs up

EDIT: OK, I checked it and the problem were the single quotes. As a rule of thumb: Don't ever use single quotes unless you really really know what you are doing. Just let DIFFCMD and MERGECMD look like described above.

@smithed
Copy link

smithed commented May 22, 2014

Hey, just saw your edit. I did replace all the $VAR with "${VAR}" so that should all be covered. I'll take another look at diff and merge at some point soon.

@smithed
Copy link

smithed commented Jun 26, 2014

OK, I give up.

I pushed some changes which make everything work perfectly fine on the command line, and it also seems to work from sourcetree...but it only works with system, because I hardcoded the paths as /usr/local/bin/.....I can't make it work any other way.

Sourcetree works with \Git\local\bin\LVCompareWrapper.sh "$LOCAL" "$REMOTE" and ...\LVMergeWrapper.sh "$BASE" "$REMOTE" "$LOCAL" "$MERGED"

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

No branches or pull requests

2 participants