Git - this time for real :)

2010-01-04
2013-06-12
  • Marko Bozikovic

    Marko Bozikovic - 2010-01-04

    Hi all,

    I have recreated Git repositories both here and on GitHib. Sorry for the inconvenience, but I wanted to make a clean jump.

    The committed version contains the latest fix for Win7 and will probably be released as build 146…

    Cheers,<br>
    Marko

     
  • Kirill

    Kirill - 2010-01-05

    Marko,

    That's great news!

    What is your git config core.autocrlf?

    The reason is I just cloned your github repo with git config core.autocrlf true and have

    $ git status
    # On branch master
    # Changed but not updated:
    #   (use "git add <file>…" to update what will be committed)
    #   (use "git checkout - <file>…" to discard changes in working directory)
    #
    #       modified:   Console/AnimationWindow.cpp
    #       modified:   Console/AnimationWindow.h
    #       modified:   FreeImage/FreeImage.h
    #       modified:   FreeImage/FreeImagePlus.h
    #       modified:   setup/config/console.xml
    #
    no changes added to commit (use "git add" and/or "git commit -a")

    And if I check the line endings of those files they all appear to be  (according to vi). If I do

        rm -rf * # all but .git
        git config core.autocrlf false
        git checkout .

    Then those files still appear to have DOS line endings, which makes me think that they have CRLF in the repo (which is sort of not recommended by Git gurus).

    I don't think it's a big deal, especially due to a doubtful value of their history, just commit and push them:

        git commit -a -m "Fix CRLF" && git push

    On the other hand, I've recently read somewhere that git svn does not work very well with autocrlf true. Which is completely irrelevant, given that you've switched for real now ;)


    Kirill.

     
  • Marko Bozikovic

    Marko Bozikovic - 2010-01-05

    Kirill,

    That's the scenario I was talking about in our recent conversation…

    I had that happen yesterday while cloning Console's SVN repo. Cloned Git repo had its autocrlf set to false, while global autocrlf is set to true.

    I deleted everything except for the .git directory, set autocrlf to <i>true</i> and ran <i>git checkout .</i> After that, I saw only console.xml as modified. I deleted everything again and did another checkout and saw those 5 files as modified. I deleted everything once more, did yet another checkout and then everything appeared fine (no modified files were reported)

    I also tried checking them out with autocrlf set to false and those 5 files had CRLF lineendings.

    I have also cloned my local repo to another dir and now I see these 5 files as modified. As you have suggested, I'll do a commit and a push and stop worrying about it :-)

    Cheers,<br>
    Marko

     
  • Kirill

    Kirill - 2010-01-05

    Marko,

    seems like b7c170ed5ec4 rolls back 95e9e774111.

    With core.autocrlf false, checking out master makes those 5 files to have CRLF. But checking out master~1 produces those files with LF (all according to vi).

    So, the suggestion is: remove the last commit:

        git reset -hard master~1 # reset your local repo and work dir to the previous commit
        git push -f # normally push would not work, hence -f

    That would probably upset a lot of people who have cloned your new repos already. But it will save us some grief in the long run.


    Kirill.

     
  • Marko Bozikovic

    Marko Bozikovic - 2010-01-06

    Kirill,

    This is what has happened: I've noticed that these 5 files have been cloned from SVN repo with CRLFs. I cloned my local repository and fixed line endings in the cloned repository. After that, I pushed the clone back to my original local repo (I think that there was a warning about pushing things into a non-bare repository).

    After that, TortoiseGit reported these 5 files changed in the working dir. I didn't check and simply committed those as well (obviously, that was the wrong move)

    I tried removing the last commit and pushing the changes, but SF's git refuses the push, even when I use -force. Maybe it's a good thing, since I read that forcing a push is not a good idea.

    It might be better if I do a checkout with autocrlf set to false, change line endings on those files and do another commit/push.

    What do you think?

    Cheers,<br>
    Marko

     
  • Kirill

    Kirill - 2010-01-06

    Well, yes. If SF does not allow forced push then another commit is pretty much the only option.


    Kirill.

     
  • Marko Bozikovic

    Marko Bozikovic - 2010-01-10

    j-pimp,

    Sorry for not answering your pull requests, as you can see, I was battling it out with Git :-)

    Would it be too much trouble for you to re-apply your changes to the recreated Console repository?

    I see that you added error messages to report configuration file loading problems and I do plan to add those.

    I also agree we need to improve the way Console resolves config file paths… Basically, Console will be used in one of the two scenarios: a portable app (console.xml will be in the same dir as console.exe) and a 'regular' app (console.xml will be in APPDATA dir)

    I don't like the idea of embedding the default config to console.exe resources, since every change in the default config means a recompile. I'd prefer if we shipped the default config in a file named something like console-default.xml. Upon startup , Console would check if there's a console.xml in the EXE dir. After that, it would check the APPDIR. If it doesn't find console.xml in either directory, it would copy console-default.xml to console.xml in the APPDIR directory and use that. Of course, cmd-line option would always override.

    Kirill, j-pimp, what do you think?

    Cheers,<br>
    Marko

     
  • Justin Dearing

    Justin Dearing - 2010-01-10

    Marko,

    Thanks for your reply. I can reapply my patches to the new repo. It might take some time to sort it all out, but nothing winmerge can't handle.

    I am not sure I understand your concern with  embedding the default config file. Perhaps you don't understand me. One would still be included with the distribution as a plain text file. The program would read that config file. However, in case that config file was not found, instead of crashing a default one would be created from the embedded resource stream.

     
  • Kirill

    Kirill - 2010-01-11

    Marko,

    nitpick: technically Justin did not need to rebase his patch, you could pull it any way (and given the size of the commit, you probably could apply it manually and just give Justin a credit in the log :)

    I agree with Justin about inclusion of a "bare-minimum, default" config into the resources to not silently crash on startup. The point is what if somebody removes everything except console.exe. The missing FreeImage and FreeImagePlus and ConsoleHook will probably be reported. However missing console.xml - or console-default.xml - will probably not.

    If you absolutely don't want to include it into resources, you can probably set up SettingsHelper in such a way that even if it can't read console.xml, it will provide safe default settings so Console can operate. And if the configuration can't be read from the disk, you might want to inform the user about this sad fact. Now, you might find it difficult to save those default, in-memory settings to an .xml file. In this case, bare-minimum XML file in resources may help.


    Kirill.

     
  • Marko Bozikovic

    Marko Bozikovic - 2010-01-11

    Ok,

    I propose the following: a default console.xml will be embedded as a resource. A future setup procedure will offer 'regular' and 'portable' installations. A portable installation will create the default console.xml in the destination directory. A regular installation will create console.xml in the APPDATA directory.

    Now, when Console is started, it will first look for console.xml in the exe directory and if it finds it, it will continue, assuming it runs as a portable installation. If it doesn't find console.xml, it will try to load it from APPDATA directory and continue as a regular installation. A command line option will always override.

    The question remains what to do if no config file is found. If it was passed in the command line option, Console should report an error and exit. Otherwise, I think it's best if it created console.xml from the embedded resource and saved it in APPDATA directory. This way we avoid problems with write permissions if Console is installed to Program Files.

    Cheers,
    Marko

     
  • Kirill

    Kirill - 2010-01-12

    Marko
    A regular installation will create console.xml in the APPDATA directory

    I totally disagree with that. The problem is: it will be created in the APPDATA of a user, who's installing the Console, which is probably an admin who wants to customize settings for all the users. So, IMHO, the installation should just install console.xml in the destination directory. It may or may not come from the resources.

    Marko
    when Console is started, it will first look for console.xml in the exe directory and if it finds it, it will continue, assuming it runs as a portable installation

    I disagree with this one as well. The current order - look in APPDATA, then look in GetModulePath(NULL) - works significantly better. For example, after an admin installed Console and customized the settings, the user will use these settings until she decides to customize them further and, if she does not have permissions to update the installed xml, saves them in her own APPDATA. After that the user will load her customized settings from APPDATA. Thus, I vote to keep the current implementation .

    Marko
    If it was passed in the command line option, Console should report an error and exit.

    Agree.

    Marko
    Otherwise, I think it's best if it created console.xml from the embedded resource and saved it in APPDATA directory

    Well… I think it should try to save it in GetModulePath first. And if failed, save it in APPDATA. But in oppose to above points, I can see why saving in APPDATA may work better.

    And a breakthrough idea: how about we continue this discussion in a Feature Request or, at least, in a separate forum thread?


    Kirill.

     
  • Marko Bozikovic

    Marko Bozikovic - 2010-01-12

    Kirill,

    I've opened a new dev forum, let's move the discussion there…

    Cheers,
    Marko

     
  • Kirill

    Kirill - 2010-01-12

    Marko,

    by the way I did not mean the new forum :) I meant just another thread in this forum.


    Kirill.

     
  • Marko Bozikovic

    Marko Bozikovic - 2010-01-12

    I thought it would be a good idea to have a separate forum for development discussions :-)

     

Log in to post a comment.

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks