WeakEvents catch with tests

Some time ago I wrote about weak events including non-standard ones. While this post is not totally dependent on weak events, I will assume you’re acquainted with MakeWeak method.

So, suppose you have some service that publishes an event, like that:

        public class SomeService
        {
            public event EventHandler TestEvent;

            internal void RaiseTestEvent()
            {
                var handler = TestEvent;
                if (handler != null) handler(this, EventArgs.Empty);
            }
        }

And you’re writing new class that will subscribe to this event weakly and re-raise it (possibly under some specific conditions). Extremely simplified it could look like this:

        public class Sut
        {
            public Sut(SomeService helper)
            {
                helper.TestEvent += new EventHandler(OnEvent).MakeWeak(eh => helper.TestEvent -= eh);
            }

            public event EventHandler TestEventReraised = delegate { };

            private void OnEvent(object s, EventArgs args)
            {
                TestEventReraised(this, EventArgs.Empty);
            }
        }

So far, so good. Now naturally you want to test if your Sut does what it should:

        [Test]
        public void SutReRaisesEventCorrectly()
        {
            var helper = new SomeService();
            var sut = new Sut(helper);
            bool raised = false;
            sut.TestEventReraised += (s, e) => raised = true;
            helper.RaiseTestEvent();
            Assert.IsTrue(raised);
        }

Seems logical, isn’t it? You may be surprised, but if you run this test in Release mode, it will fail randomly. Most of the time it will pass, but if you use build server and have many tests in your project – this one will fail eventually. If you have been reading my blog for some time, you may recognize the symptoms as they are very similar to this case. Really, if you change test like below it will fail consistently in Release mode (in Debug it will still succeed always):

        [Test]
        public void SutReRaisesEventCorrectly()
        {
            var helper = new SomeService();
            var sut = new Sut(helper);
            bool raised = false;
            sut.TestEventReraised += (s, e) => raised = true;

            GC.Collect();

            helper.RaiseTestEvent();
            Assert.IsTrue(raised);
        }

Really, sut variable’s value is eligible for collection at highlighted line, so it is collected (our event has weak subscription, remember?), and event is never raised.

As it takes much attention to not make such errors, I’d suggest to make sut variable not local for test method but rather class variable. Then it will never be eligible for collection in such cases and you will not write randomly failing tests. Like that:

    [TestFixture]
    public class WeakEventsCheck
    {
        private Sut _sut;

        [Test]
        public void SutReRaisesEventCorrectly()
        {
            var helper = new SomeService();
            _sut = new Sut(helper);
            bool raised = false;
            _sut.TestEventReraised += (s, e) => raised = true;

            GC.Collect();

            helper.RaiseTestEvent();
            Assert.IsTrue(raised);
        }
    }

MSTest broke compatibility. Again

Visual Studio 2012 Update 2 has changed something in MSTest so that it broke compatibility with ReSharper. From the description it seems it forced JetBrains to release next version on ReSharper, so users of R# had problems only for one week. Great work, actually: I bet it is not a trivial task to establish development process that allow something of R# caliber to be released in one week.

Though it is another nail to MSTest coffin, IMO. It is not only stuck in place and not evolving (gosh, they can’t make asserts for exceptions or even fix bug that ExpectedException attribute don’t check message even if asked to for years!), but actually disrupt work process from time to time with breaking changes, strange SEH exceptions or incompatibilities.

I’m extremely happy we were able to abandon MSTest and adopt NUnit.

If you have new project and deciding which unit-test framework to use – do yourself a favor and avoid MSTest as much as possible.

EnumerateChain extension method

Very small, simple and extremely useful method:

        public static IEnumerable<T> EnumerateChain<T>(this T start, Func<T, T> next)
            where T : class
        {
            while (start != null)
            {
                yield return start;
                start = next(start);
            }
        }

How to use it you may ask. Suppose (just for the sake of example) you have some tree structure, with Name and Parent properties in each node. And you want to get path in tree of some given node in form of node names separated by slashes. Nothing could be easier:

public static string GetPathInTree(Node node)
{
    var names = node.EnumerateChain(n => n.Parent).Reverse().Select(n => n.Name);
    return String.Join("/", names);
}

Basically with this method you can traverse any structure as if it was normal IEnumerable. Very convenient, really

Setting git to work behind NTLM-authenticated proxy: cntlm to the rescue

Our office has corporate proxy – and that’s pretty common as far as I know. And it happened that our proxy has NTLM authentication which is not very common/popular among linux users (that’s an understatement, I know). So git and msysgit don’t support it out-of-the-box.

Thus, if you try to clone something over SSL behind such proxy, say, from github – you’ll see something like below:

$ git clone https://github.com/libgit2/libgit2sharp.git
Cloning into 'libgit2sharp'...
error: Failed connect to github.com:443; No error while accessing https://github.com/libgit2/libgit2sharp.git/info/refs
fatal: HTTP request failed

And it will take almost forever by the way (dozen of minutes in my case).

Well, to solve the problem you may specify proxy manually like that:

$ git config http.proxy http://domain\\\user:password@proxy.MyCorp.com:8080/

Note three backslashes, it is not a mistake.

But you already see the problem, don’t you? git config is saved in plaintext and I’m not very like the idea to store my password like that, even if in some location where nobody has access rights (domain administrators could see everything in the domain, remember?). And passing token instead of login/password is beyond msysgit abilities.

You may think: if I’m able to get to github in browser, even over SSH, then why git can’t? Maybe there’s some way to intercept git’s queries and wrap them through correct proxy/auth headers? And you’d be right: there is tool capable of that, name is cntlm.

Some time ago cntlm had have a bug preventing it from working in exactly the same situation: http://stackoverflow.com/questions/6345104/proxy-through-cntlm-why-git-clone-hangs

Fortunately, now the bug is fixed (at the time of writing, latest cntlm version was 0.92.3). So, what it is and how you can use it?

Cntlm is the proxy itself, that you run on your local box and when it receives a request – it re-sends it to so-called ‘parent proxy’ (which is exactly your NTLM-authenticated beast) with correct auth headers.

First, you need to download distributive and install it. In the installation folder you will find cntlm.ini file, which you want to edit. You have to set correct Username, Password and auth tokens and remove Password field (as you don’t want to store password in the plaintext). Then run command line and run these:

$ cntlm.exe -H -u username -d domain
Password:
PassLM          1AD35398BE6565DDB5C4EF70C0593492
PassNT          77B9081511704EE852F94227CF48A793
PassNTLMv2      B78FD04127AEDF090C1F7121ED002A4D    # Only for user 'username', domain 'domain'

Just paste these PassNTLMv2 string to the cntlm.ini (don’t forget to type your own username and password instead of the ‘username’ and ‘password’ strings above). Don’t touch PassLM or PassNT tokens if your system supports NTLMv2 because NT and LM are not secure.

It would be all of the configuration. Run ‘cntlm.exe’. You could try ‘cntlm.exe -v’ to activate verbose mode and have much info on the console and see if something went wrong. If you run cntlm without params – it will go to daemon mode and will silently run in background, listening for 3128 port (by default – can be changed in cntlm.ini easily). And by default it won’t accept connections from others non-local programs, so that your box will not become a gateway for everything possible if someone knows you’re running cntlm (that can be changed as well).

Now we need to make git use it. It’s easy:

$ git config --global http.proxy localhost:3128

$ git clone https://github.com/libgit2/libgit2sharp.git
Cloning into 'libgit2sharp'...
remote: Counting objects: 7992, done.
remote: Compressing objects: 100% (2160/2160), done.
remote: Total 7992 (delta 5944), reused 7634 (delta 5680)
Receiving objects: 100% (7992/7992), 39.44 MiB | 217 KiB/s, done.
Resolving deltas: 100% (5944/5944), done.

That’s it. In case you’re interested in detail info about cntlm command-line parameters, here it is.

Property Injection for infrastructural dependencies in practice, part 2

In the first part I introduced RequiredPropertyAttribute and integrated it with Windsor. And the problem of preventing manual instantiation of such classes (because manually nobody is forcing us to treat such properties as required in any way) remained open.

So, the best thing possible from my point of view would be compiler emitting warning each time we create such classes. Then you will have a clear sign you’re doing something not-intended. Or, if you have “Treat compiler warnings as errors” – it will not allow you to even compile such code.

I believe with Roslyn it is not so hard to write custom inspection for this and warn user whenever required. But for now it is not that easy unfortunately. At least I haven’t found better way than described below (if you know one – let me know, please).

The closest thing (actually, surprisingly close to our needs) is ObsoleteAttribute on the class constructor. It has several advantages:

• compiler emits warning only if it is used, so we don’t need to analyze code ourselves;
• it is totally ignored by IoCC and other reflection-based approaches;
• we may customize warning text to let user know that instantiating of a class should be done via IoCC rather then manually;
• compiler enforces “viral propagating” of the attribute.

Let me elaborate a bit on the last point. Suppose you have this class:

    public class A
    {
        [Obsolete("You should not call this contructor manually. It should be called only by IoC container to avoid missing dependencies")]
        public A() {}
    }

And someone writes that one:

    public class B : A {}

Oops. Immediate warning: “‘A.A()’ is obsolete”. In order to have such class without warning you have to write that code:

    public class B : A
    {
        [Obsolete]
        public B() {}
    }

And that is a good thing, obviously.

Now, some disadvantages clearly present as well. First off, it is a hack and using Obsolete in not-intended way. Also you have no means to avoid setting this long string line if you want clear warning message (ObsoleteAttribute class is sealed unfortunately). And finally, if you’re developing some library and making it backward-compatible – there’re good chances you need Obsolete for your own legitimate use. Thus said, if you want to incorporate this approach – you will be marking as Obsolete some internal implementations which probably will not be seen by user of your library anyway.

And obviously, this approach doesn’t defend you against reflection. But reflection is absolutely another story where you have much less guarantees by default, so I think it is ok.

Though I’m not absolutely content with this solution – it works and I can’t think out something better right now. And I can move my app to property injection more or less sure that required properties will be really required, and these requirements would be checked as early as possible.

Property Injection for infrastructural dependencies in practice, part 1

Inspired by this post I tried this approach myself. If you haven’t read it yet – it’s time to spend several minutes there.

The main reason I was reluctant to use “infrastructure dependencies as properties, application-level dependencies as ctor arguments” is that it is very easy to miss the required property and find oneself one day in debugging because of some silly configuration error or forgetting to set a property. I strive to use static checking as much as possible and detect errors in compile-time or build-time (on build server where some additional checks are taking place).

On Inversion of Control Container (IoCC) side it can be solved – as Krzysztof suggested – via configuring so that it treats them as required. Here I will show things using Castle Windsor 3.0, but the approach overall is not specific to concrete container.

The default way in Windsor to make property required is at configuration time shown here. Personally I don’t like that very much, because in this way you need to find registration and look through it in order to understand if some property will be filled always or not. It is much cleaner and easier to read if it is immediately obvious right in the property declaration place, thus making custom attributes natural choice. As far as I’m aware Windsor don’t have built-in attribute for that, so let’s make our own.

It is easy if you know where to look. I have to admit I spent about an hour to find the place and another 15 minutes to write implementation and tests for it.

Here you go:

    [AttributeUsage(AttributeTargets.Property)]
    public sealed class RequiredPropertyAttribute : Attribute
    {
    }

    public class RespectRequiredPropertiesContributor : IContributeComponentModelConstruction
    {
        public void ProcessModel(IKernel kernel, ComponentModel model)
        {
            var markedProperties = model.Properties
                .Where(p => p.Property
                                .GetCustomAttributes(typeof(RequiredPropertyAttribute), true)
                                .FirstOrDefault() != null);
            foreach (var p in markedProperties)
            {
                p.Dependency.IsOptional = false;
            }
        }
    }

And here are the tests:

    public class Dependency
    {
    }

    public class ClassWithoutRequiredProperties
    {
        public string StringProperty { get; set; }
    }

    public class ClassWithRequiredProperty
    {
        [RequiredProperty]
        public Dependency Dependency { get; set; }
    }

    public class ClassWithInheritedRequiredProperty : ClassWithRequiredProperty
    {
    }

    [TestFixture]
    public class WindsorRespectRequirementPropertiesTests
    {
        private IWindsorContainer _windsor;

        [SetUp]
        public void Setup()
        {
            _windsor = new WindsorContainer();
            _windsor.Kernel.ComponentModelBuilder.AddContributor(new RespectRequiredPropertiesContributor());
        }

        [Test]
        public void NoPropertiesAreRequired_ComponentCreated()
        {
            _windsor.Register(Component.For<ClassWithoutRequiredProperties>());
            var obj = _windsor.Resolve<ClassWithoutRequiredProperties>();
            Assert.NotNull(obj);
            Assert.IsNull(obj.StringProperty);
        }

        [Test]
        public void SomePropertiesAreRequired_DependencyMissing_Throws()
        {
            _windsor.Register(Component.For<ClassWithRequiredProperty>());
            Assert.Throws<HandlerException>(() => _windsor.Resolve<ClassWithRequiredProperty>());
        }

        [Test]
        public void SomePropertiesAreRequired_DependencyExists_ResolveSuccessfull()
        {
            _windsor.Register(Component.For<ClassWithRequiredProperty>());
            _windsor.Register(Component.For<Dependency>());
            var obj = _windsor.Resolve<ClassWithRequiredProperty>();
            Assert.IsNotNull(obj);
            Assert.IsNotNull(obj.Dependency);
        }

        [Test]
        public void SomePropertiesAreRequiredInBaseClass_DependencyMissing_Throws()
        {
            _windsor.Register(Component.For<ClassWithInheritedRequiredProperty>());
            Assert.Throws<HandlerException>(() => _windsor.Resolve<ClassWithInheritedRequiredProperty>());
        }

        [Test]
        public void SomePropertiesAreRequiredInBaseClass_DependencyExists_ResolveSuccessfull()
        {
            _windsor.Register(Component.For<ClassWithInheritedRequiredProperty>());
            _windsor.Register(Component.For<Dependency>());
            var obj = _windsor.Resolve<ClassWithInheritedRequiredProperty>();
            Assert.IsNotNull(obj);
            Assert.IsNotNull(obj.Dependency);
        }
    }

Now, you may use this attribute and Windsor will treat both constructor dependencies and marked property dependencies equally required.

But still, there’s one problem. What if someone will create some class by manually calling the constructor? Well, maybe he even will be attentive enough and checks which properties are required and sets them properly. But still such code would be defenseless against change in the class being created. What happens if later another (not so attentive) developer adds some required property? Clearly, he needs to check every site where class is created manually and satisfy that new dependency. In other words, allowing to create such classes manually is a way to make IoCC’s work by hand. Clearly not a good thing – if we have IoCC – we should use it, otherwise why it clutters the code?

But how one can prevent creating some classes in code, but still allow IoCC to create them? I will describe one way in part 2 shortly. It is not perfect, but it works and makes unintended error much less likely.

Visual Studio 2012: several rows of tabs

There’s very useful ability in Visual Studio 2012 that is disabled be default – I don’t know why. It can be enabled through Tools -> Options -> Environment -> Tabs and Windows -> Show pinned tabs in separate row.

For example, this is VS window with default settings:

You can see that all tabs placed to single row. And some of them even hidden – to look at them – you should click drop down button at the right side of tabs row and select tab you’re looking for. Well, when you need more than four or five tabs at once (maybe a little more depending on your display size) – it quickly becomes very inconvenient experience.

After you turn on “Show pinned tabs in separate row” option… well, nothing changed. That’s because we haven’t pinned any tabs yet, of course. When you do – you’ll see something like that:

And even better: your pinned tabs will never hide under drop down button! When you pin more tabs than one row can take – another row appears:

It is almost as if Visual Studio 6.0 is back! Do you remember that VS 6.0 has multi-row tabs from the start and then in VS 7.0 this useful feature was removed for some reasons? Well, now it is back. And in conjunction with Preview Tab (you can find more info about it here) it shines even better than before.

Bug in C#/IL optimizations logic? o_O

To be short: after half a day of minimizing bug case I got this code:

    [TestFixture]
    public class Tests
    {
        private class Relay
        {
            public Action Do { get; set; }
        }
        
        [Test]
        public void OptimizerStrangeness()
        {
            var relay = new Relay();
            var indicator = 0;
            relay.Do = () => indicator++;
            var weak = new WeakReference(relay);
            
            GC.Collect();

            var relayNew = weak.Target as Relay;
            if (relayNew == null) Assert.Fail();
            relayNew.Do();
            Assert.AreEqual(1, indicator);
        }
    }

Here is StackOverflow question on this topic.

Compiled under Debug mode test passes. Under Release mode it fails on highlighted line. relayNew variable is null there. But we still have relay variable in scope and haven’t assigned null to it. We still have strong reference! So, what’s happening?

I can only speculate, but my assumption is that optimizer was designed to follow such logic:

1. relay variable is not accessed after assigning weak variable on line 15 (it could be easily proven via analyzing IL);
2. we are about to call GC.Collect() manually, so probably we want to reclaim as much memory as possible;
3. optimizer can “help” us by marking relay as not accessible and GC it with other garbage;

But it changes the visible behavior of the compiled code, so I assume it is a bug.

I checked generated IL and it is the same for Debug and Release mode (at least in regard to variable eligibility for GC). The only difference seems to be assembly-level attribute:

• Debug mode:

  [assembly: Debuggable(DebuggableAttribute.DebuggingModes.Default
                      | DebuggableAttribute.DebuggingModes.DisableOptimizations
                      | DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints 
                      | DebuggableAttribute.DebuggingModes.EnableEditAndContinue)]
  

• Release mode:

  [assembly: Debuggable(DebuggableAttribute.DebuggingModes.IgnoreSymbolStoreSequencePoints)]
  

According to documentation it is related to JIT-optimizations, so my final assumption is that JIT-compiler makes things he thinks of as safe, but which are not.

Another indirect proof is that if you change code like below (only highlighted line added), test will pass in both Debug and Release modes:

        [Test]
        public void OptimizerStrangeness()
        {
            var relay = new Relay();
            var indicator = 0;
            relay.Do = () => indicator++;
            var weak = new WeakReference(relay);
            
            GC.Collect();

            var relayNew = weak.Target as Relay;
            if (relayNew == null) Assert.Fail();
            relayNew.Do();
            Assert.AreEqual(1, indicator);
            Assert.IsNotNull(relay);
        }

Here, optimizer can’t deduce that relay‘s use is trivial and cannot optimize it away before last line passed.

The problem is reproducible on .NET 2.0, .NET 4.0 and .NET 4.5 equally.

P.S. Actually I found the problem in real code, where I have weak event handlers that treated this dead weak reference as a message that event’s target has been GCed already and were unsubscribing from event source. And on top of it some ObservableCollection<T> that were using these weak events. It wasn’t an easy task to localize the problem. In fact even reproducing test failure was not trivial, because I usually build in Debug mode at my box and good long time was wondering why tests pass on my machine, but were failing on three different build agents.
Phew. At least it can be fixed without re-architecturing half of the app.

Update. Here is the comment I found in .NET Framework sources for GC.KeepAlive(object obj) method:

    // This method DOES NOT DO ANYTHING in and of itself.  It's used to
    // prevent a finalizable object from losing any outstanding references 
    // a touch too early.  The JIT is very aggressive about keeping an 
    // object's lifetime to as small a window as possible, to the point
    // where a 'this' pointer isn't considered live in an instance method 
    // unless you read a value from the instance.  So for finalizable
    // objects that store a handle or pointer and provide a finalizer that
    // cleans them up, this can cause subtle ----s with the finalizer
    // thread.  This isn't just about handles - it can happen with just 
    // about any finalizable resource.
    // 
    // Users should insert a call to this method near the end of a 
    // method where they must keep an object alive for the duration of that
    // method, up until this method is called.  Here is an example: 
    //
    // "...all you really need is one object with a Finalize method, and a
    // second object with a Close/Dispose/Done method.  Such as the following
    // contrived example: 
    //
    // class Foo { 
    //    Stream stream = ...; 
    //    protected void Finalize() { stream.Close(); }
    //    void Problem() { stream.MethodThatSpansGCs(); } 
    //    static void Main() { new Foo().Problem(); }
    // }
    //
    // 
    // In this code, Foo will be finalized in the middle of
    // stream.MethodThatSpansGCs, thus closing a stream still in use." 
    // 
    // If we insert a call to GC.KeepAlive(this) at the end of Problem(), then
    // Foo doesn't get finalized and the stream says open. 
    [System.Security.SecuritySafeCritical]  // auto-generated
    [ResourceExposure(ResourceScope.None)]
    [MethodImplAttribute(MethodImplOptions.InternalCall)]
    [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] 
    public static extern void KeepAlive(Object obj);

Also you may be interested in this excerpt from C# Language Specification, section 3.9:

If the object, or any part of it, cannot be accessed by any possible continuation of execution, other than the running of destructors, the object is considered no longer in use, and it becomes eligible for destruction. The C# compiler and the garbage collector may choose to analyze code to determine which references to an object may be used in the future. For instance, if a local variable that is in scope is the only existing reference to an object, but that local variable is never referred to in any possible continuation of execution from the current execution point in the procedure, the garbage collector may (but is not required to) treat the object as no longer in use.

So, now I’m convinced now that it is intended behavior. Good for performance, bad for clarity. Well, now I know that in such situations I have to remember to put GC.KeepAlive(relay) in the end (instead of NotNull assertion, that is meaningless in production code of course). Better I won’t forget this

Hail the unit-tests and hail the build server running them in Release mode!

My environment for day-to-day work with git-tfs

I’ve been asked many times to describe how things are organized at our project with git-tfs, so it’s the time to write it down – repeating these things over and over isn’t effective at all.

So, below I will describe how things are arranged to make TFS2010, msysgit and git-tfs work together.

First, setup involves several physical boxes, so lets name them at once:

1. DevA, DevB – two developer’s boxes;
2. GitServer – Windows Server 2008 R2 box with IIS – hosts central git repository, exposes git repositories over HTTP and has scheduler task to synchronize central git repository with TFS;
3. TfsServer – remote host with TFS Server 2010 (it is our corporate beast and I don’t have administrative access there);

I will assume you already installed git and git-tfs to your machines. So, let’s start.

 

1. Cloning TFS repository first time

First, you have to clone repository from TFS somewhere. On your DevA box open bash console, go to the folder where you want sources reside and execute this:

git tfs clone -d http://TfsServer:8080/tfs $/ProjectA/Src proj-git

or maybe, if you don’t interested in full history and want to have just last TFS sources state:

git tfs quick-clone -d http://TfsServer:8080/tfs $/ProjectA/Src proj-git

It may take up to several hours if you have large repository and slow connection. If everything is fine – folder proj-git will appear there. Inside are your working copy with latest TFS version and folder .git with configuration.

 

2. Setting up GitServer and auto-update from TFS

git-tfs doesn’t work with bare repositories (at least it was so at the time I followed its development), thus we will need two repositories for each TFS one: first will be bare, and accessible through HTTP; and second one would be private – there we will setup scheduled fetching from TFS server, each time new changes are found in TFS – we will push them to first repository. To distinguish these two I will refer to first as Central and second as Working.

1. Install IIS server role
2. Install GitWebAccess. As I wrote before last version seems broken, so you can download my working version. Unpack it to C:\inetpub\wwwroot\gwa, and follow this manual. I will suppose you created folder C:\GitRepos for your git repos as suggested in this manual.
3. Create new folder for Working repo somewhere. For this example it would be C:\working
4. Copy content of proj-git folder (where you’ve cloned TFS repository on 1st step before) to C:\working and execute

   git checkout -B master tfs/default
   

   thus master branch will be set at your last TFS commit

5. Add C:\working\.gitignore file if it is not present with these lines:

   /update
   /log.txt
   

   If file is present – just add such lines there. update is the script we will write next and log.txt… well, it is log of updating. As updating will be executed by scheduler – it is the only way to know what’s happening.

6. Now we will need bare repository in the C:\GitRepos\Project.git. Execute from C:\GitRepos these command:

   git clone --bare /c/working Project.git
   git remote rm origin # we don't need origin here
   

   If it was done correctly – you should have your repository accessible by HTTP through GitWebAccess.

7. Now it’s the time to setup update in Working repository. The convention I embrace is that master branch of my repository will always follow TFS repository line. Thus, no one except update script should push it directly and it will never have changes conflicting with TFS data.
   You need to setup origin remote in the Working repo in order for script below to work:

   git remote add origin /c/GitRepos/Project.git
   

   Next, create file C:\Working\update and put these lines to it:

   #!/bin/sh
   
   check_err()
   {
   	# parameter 1 is last exit code
   	# parameter 2 is error message that should be shown if error code is not 0
   	if [ "${1}" -ne "0" ]; then
   		cat '~temp.log'
   		echo ${2}
   		rm -f '~temp.log' > /dev/null
   		exit ${1}
   	fi;
   	rm -f '~temp.log' > /dev/null
   }
   
   echo "$(date) Update launched"
   
   if [ ! -z "$(git status --porcelain)" ]; then
   	echo "$(date) Your status is not clean, can\'t update"
   	exit 1;
   fi;
   
   echo "$(date) Pulling from central repo first to avoid redundant round-trips to TFS..."
   git pull origin master:master > '~temp.log'
   check_err $? "Pulling from central repo failed"
   
   echo "$(date) Pulling from TFS..."
   git tfs pull -d > '~temp.log'
   check_err $? "Pulling from TFS resulted in error";
   
   local_commits_to_push="$(git rev-list master ^origin/master)"
   if [ -z "$local_commits_to_push" ]; then
   	echo "$(date) Central repo is up-to-date, nothing to push"
   else
   	echo "$(date) Pushing updates to central repo"
   	git push origin master > '~temp.log'
   	check_err $? "Push to central resulted in error";
   fi;
   

   You may check it – just execute update from bash console. Your latest updates should be fetched from TFS. In case there were some changes – they should be also pushed to Central‘s master branch.
   It is simplified version of the script from this post. Also there you can find how to create scheduler task to execute this update script every 5 minutes – do it now.

 

3. Create rcheckin and reap the benefits

Now, back to the developer’s environment. We want to use that auto-updating central repository effective, don’t we? So:

• We don’t need to use git tfs fetch at all anymore: we may limit ourselves to git fetch because latest code is always in the Central git repository.
• When git tfs rcheckin is executed – it fetches latest changes from TFS. We need to reuse as much as possible from already fetched changes. So, we need to fetch manually from Central before executing git tfs rcheckin and somehow make git-tfs mark it as correct TFS-fetched commits.
• And finally, after we checkined something to TFS – we want to push it to Central, so auto-updater won’t fetch what we already have – it could have significant performance impact in case of large commits, or long line of commits with slow connection. In such case we may even want to disable auto-updater for some time until we push things to Central and then just re-enable it.

So, in order to achieve all these points – instead of using git tfs rcheckin that doesn’t know anything about our environment – we will use custom script rcheckin (do not forget to add it to .gitignore if needed):

#!/bin/sh

check_err()
{
	# parameter 1 is last exit code
	# parameter 2 is error message that should be shown if error code is not 0
	if [ "${1}" -ne "0" ]; then
		cat '~temp.log'
		echo ${2}
		rm -f '~temp.log' > /dev/null
		exit ${1}
	fi;
	rm -f '~temp.log' > /dev/null
}

if [ ! -z "$(git status --porcelain)" ]; then
	echo "Your status is not clean, can't continue"
	exit 1;
fi;

echo "Fetching origin..."
git fetch origin

if [ -n "`git rev-list HEAD..origin/master`" ]; then
	echo "origin/master has some TFS checkins that are conflicting with your branch. Please reabse first, then try to rcheckin again"
	exit 1;
fi;

echo "Marking latest TFS commit with bootstrap..."
git tfs bootstrap -d

git tfs rcheckin
check_err $? "rcheckin exited with error"

git push origin HEAD:master
check_err $? "Can't push HEAD to origin"

Well, that’s almost all.

 

4. Setup another developer environment

So, suppose DevB also needs to work with your server. Here is how he can start his work easily:

1. git clone http://GitServer:8080/project.git project
   cd project
   git tfs bootstrap -d # make git-tfs mark what is required
   

2. Get rcheckin script from previous section.
3. Work!

 

5. My common workflow

So, I’ll try to describe how I work in general, supposing I’m in my dev repository, set up as described above.

1. First, let’s check that no changes appeared while I was writing all of these: git fetch origin. OK, no changes.
2. Working, making changes hard… committing several times, squashing, rewriting etc.
3. Ready to push to TFS? Great! Let’s check again – maybe someone pushed something while I have been working: git fetch origin. Yeah, there’re some changes, origin/master has updates. Let’s rebase my work there (I don’t like merging my local work which is not seen by anyone except me – it just clutters commit graph): git rebase origin/master, maybe some conflict resolving. Hey, I’m ready to push: rcheckin.
4. Yet some work.
5. Working time is over and I have not finished my current task. I want it backed up on server, so my local machine failure can’t waste my work: git push origin HEAD:idanilov/temp. idanilov is my ‘private’ branch namespace, every developer has one. By convenience it is just backups and it is not considered published. For open-source projects you probably won’t do this, but corporative rules sometimes require some assurance that you won’t lost your work that is already paid for.

Sometimes I may switch branches, stash changes and do other git stuff, but it is not so frequent. Most often all of my workflow is fetch-rebase-rcheckin chain.

I wanted to explain yet some things – e.g., how to have a bunch of bash scripts in a way that it doesn’t get to TFS (it’s easy, just use .gitignore) and have a way to synchronize them across all developers using git (it’s much harder if you don’t want to bother running to every developer, copying new versions of scripts each time you want to correct one line in some script). Or how to automate git console, so you don’t need to type long commands every time which is pretty boring. But… this post is already pretty long, so I will do this later.

Have fun moving away from TFS with git and git-tfs!

Implicit closures in C# lambdas, part 2

Wow. I’ve just found that not all I said in first part is true (I’ve already marked wrong place so readers would not be confused). Shame on me.

My mistake was to rely on ReSharper’s warnings totally. So, in the ending of last post I had changed code to make copy of variable for closure like that:

        public static void DoSomething(object a, object b, Observer observer, Receiver r)
        {
            var receiverCopy = r;
            EventHandler action1 = (s, e) => receiverCopy.Method(a);
            EventHandler action2 = (s, e) => r.Method(b);
            observer.X += action1;
            observer.X += action2;
        }

And assumed that it is enough to convince compiler to capture variables for two lambdas in two different compiler-generated classes – based on the fact that ReSharper warning is gone. It turned out (by experiencing very real memory leak in unit-test first and then looking at generated IL to understand) absence of warning means that ReSharper is just unable to find such implicit closure, not that implicit closure is gone. Compiler is still generates single class for all these closures. And at this time it is probably an optimization from compiler’s side – namely, try to decrease number of types in the assembly by generating one less.

What I ended up with is hand-writing explicit class very similar to that in previous post. This solved problem with memory leak, and looking at IL proved that it helped. But code is now ugly. And worse – I don’t know how to spot such problems easily without thorough analysis of the code.

UPD: Pretty much the same thing in another blog post.

UPD2: As Eric answered at SO, this behavior was implemented because it was easier (for compiler developers) and part about lambda rewriting was already pretty complicated at the time. And, hopefully with Roslyn we may have better.

Also, good alternative was offered as well.