microsoft / scalar

Scalar: A set of tools and extensions for Git to allow very large monorepos to run on Git without a virtualization layer
MIT License
1.39k stars 63 forks source link

Delete most of the Scalar product code #511

Closed derrickstolee closed 3 years ago

derrickstolee commented 3 years ago

Draft only because I'm not sure if this will pass tests. The biggest unknown is whether or not the microsoft/git installer will still be downloaded during functional tests, which is the only reason the Scalar, Scalar.Common, and Scalar.MSBuild projects still exist.

Based on #505 because we need to point to main to trigger the workflows.

mjcheetham commented 3 years ago

@derrickstolee you should be able to delete the signing and MSBuild projects too.. they're only used for signing and generating code for the non-existent product respectively 😁

derrickstolee commented 3 years ago

@derrickstolee you should be able to delete the signing and MSBuild projects too.. they're only used for signing and generating code for the non-existent product respectively 😁

@mjcheetham: I'm worried that without it I won't get anything that downloads the microsoft/git package, but I'll give it a try.

derrickstolee commented 3 years ago

@derrickstolee you should be able to delete the signing and MSBuild projects too.. they're only used for signing and generating code for the non-existent product respectively 😁

@mjcheetham: I'm worried that without it I won't get anything that downloads the microsoft/git package, but I'll give it a try.

The obvious thing of deleting the project failed, so I'm going to leave that one to another PR (if at all) because it's not as simple as the other projects.

derrickstolee commented 3 years ago

I'm rubber stamping this. I looked thru things and it all looks good. I'm not sure what's happening with the builds failing on 1 machine and/or the signing thing.

In my opinion, our functional tests are a little more flaky in the C port, so we will want to resolve some of those issues in the coming weeks.

dscho commented 3 years ago

our functional tests are a little more flaky in the C port

I fear you are right. My take on it is that it was my mistake to rely on the global config so much. In typical usage scenarios, this does not matter: users do not concurrently register a dozen enlistments. But the Functional Tests do.

To some extent, the problem was alleviated by my patch that asks for a back-off strategy to be used when creating that write lock.

I expect the remaining problems to be rooted in the lack of a read lock. That is, when we read the global Git config, we do not expect there to be any problems.

And those problems would only occur on Windows, anyway: on Linux/macOS, the rename() that commits the write lock is atomic. Either you get the old, or you get the new version of the config.

In contrast, on Windows there is the potential that a different process is holding a lock on that file (I still suspect Defender to interfere here, but it could potentially be the MoveFilesExW() call that might not be lock-free).

My best guess is that we should actually implement a back-off strategy with a very short time-out in mingw_open() and mingw_fopen(). It could be either modeled after the strategy in lock_file_timeout(), or even preferable: we should attempt to refactor that strategy into a reusable form and use that. I could imagine something like this:

struct back_off_t back_off;

back_off_init(&back_off, time_out_ms);
do {
  file = _wfreopen(wfilename, wotype, stream);
  if (file || errno != EPERM)
    break;
} while (back_off_sleep(&back_off));

This is still a bit clunky, and could potentially be wrapped in a macro, somewhat similar to how SWAP() is implemented.