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

StartUpToDateCheck(null, 1) throws ArgumentException #471

Closed
jcansdale opened this issue May 27, 2015 · 14 comments
Closed

StartUpToDateCheck(null, 1) throws ArgumentException #471

jcansdale opened this issue May 27, 2015 · 14 comments
Labels
Milestone

Comments

@jcansdale
Copy link

Call to IVSBuildableProjectCfg.StartUpToDateCheck(null, 1) throws an ArgumentException on VS 2015 RC F# projects. The 1 indicates the VSUTDCF_DTEEONLY flag is set.

On C# and other common project types, this call indicates whether Visual Studio thinks the project needs building. I've found it to be the quickest and most reliable way to find which projects need building. Unfortunately this method doesn't seem to be well documented.

Here is the full exception:

[11268] System.ArgumentException: Value does not fall within the expected range.
[11268] at System.Runtime.InteropServices.Marshal.ThrowExceptionForHRInternal(Int32 errorCode, IntPtr errorInfo)
[11268] at Microsoft.VisualStudio.ErrorHandler.ThrowOnFailure(Int32 hr, Int32[] expectedHRFailure)
[11268] at Microsoft.VisualStudio.FSharp.ProjectSystem.Utilities.TryGetActiveConfigurationAndPlatform(IServiceProvider serviceProvider, IVsHierarchy hierarchy, ConfigCanonicalName& configCanonicalName)
[11268] at Microsoft.VisualStudio.FSharp.ProjectSystem.ProjectConfig.IsUpToDate(OutputWindowLogger logger, Boolean testing)
[11268] at Microsoft.VisualStudio.FSharp.ProjectSystem.BuildableProjectConfig.StartUpToDateCheck(IVsOutputWindowPane pane, UInt32 options)

I think this method may have always returned false on previous versions (rather than throwing an exception). This isn't a show stopper.

@jcansdale
Copy link
Author

To repro:

  1. Install TestDriven.NET-3.8.2877 RTM
    http://testdriven.net/download_release.aspx?LicenceType=Personal

  2. Create F# class library project in VS 2015 RC with:
    module adhoc
    let hello = printfn "Hello, world!"

  3. Right click and 'Run Test(s)' anywhere on 'let hello ...' line.

This will cause 'System.ArgumentException' to be thrown and F# project won't be built before 'hello' is run. Exception will be visible in Sysinternals DebugView.

@latkin
Copy link
Contributor

latkin commented May 27, 2015

Thanks for the details. The up to date check doesn't currently pay any attention to the initial options flags, we should add proper consideration for those.

AFAIK this won't affect normal user builds, but we should do our best to fully support the various automation scenarios, too.

@enricosada
Copy link
Contributor

i can repro,

the exception is raised when solutionBuildManager.FindActiveProjectCfg is called

@jcansdale
Copy link
Author

I've just checked to see how F# projects behave in previous versions of Visual Studio. In VS 2008, 2010 and 2013, the following returns false (or rather a H result of < 0):

QueryStartUpToDateCheck(1, pfSupported, pfReady)
// 1 is the VSUTDCF_DTEEONLY flag

When this happens, TestDriven.Net assumes that the project might need building and doesn't call StartUpToDateCheck (which throws the exception in VS 2015).

Keeping this behavior consistent in VS 2015 might be a better fix?

@enricosada
Copy link
Contributor

old code:

QueryStartUpToDateCheck

  • supported = 0
  • ready = (IsInProgress()) ? 0 : 1 that's wrong, ready mean ready to start the up-to-date check, not the build itself. ready param is unused
  • return: VSConstants.S_OK ( = 0 )

StartUpToDateCheck

  • return: VSConstants.E_NOTIMPL ( = -2147467263 )

@jcansdale
Copy link
Author

Did you mix up the method names there?

Won't it have been QueryStartUpToDateCheck that returned
VSConstants.E_NOTIMPL?

On 28 May 2015 at 12:59, Enrico Sada notifications@github.com wrote:

old code:

QueryStartUpToDateCheck

  • supported = 0
  • ready = (IsInProgress()) ? 0 : 1
  • return: VSConstants.S_OK

StartUpToDateCheck

  • return: VSConstants.E_NOTIMPL


Reply to this email directly or view it on GitHub
#471 (comment)
.

@enricosada
Copy link
Contributor

@jcansdale see pr #473 .
Disabled both QueryStartUpToDateCheck ( supported = 0 ) and StartUpToDateCheck.
Now behave like the old code, and testdrive works ok 😄

@enricosada
Copy link
Contributor

@jcansdale :

Did you mix up the method names there?

Won't it have been QueryStartUpToDateCheck that returned
VSConstants.E_NOTIMPL?

nope, return OK (0), but supported false (0)
That's ok, if not supported, StartUpToDateCheck is not called with VSUTDCF_DTEEONLY

image

@enricosada
Copy link
Contributor

as a side note, options can contains flags from VsUpToDateCheckFlags2 ( vs 2012+ )

Flags passed into AreProjectsUpToDate as well as QueryStartUpToDateCheck and StartUpToDateCheck to indicate that the operation for the purpose of a particular build request is done.

  • VSUTDCF_PACKAGE A package build (to create an app package for a Windows Store app) has been requested.
  • VSUTDCF_PRIVATE A mask for any custom VS_BUILDABLEPROJECTCFGOPTS_PRIVATE flags that were specified with the build.
  • VSUTDCF_REBUILD A full rebuild has been requested.

@latkin @KevinRansom any idea if we should manage also these flags? retro-compatibility like VSUTDCF_DTEEONLY ?

@jcansdale
Copy link
Author

Excellent, thanks for checking! It was indeed the supported = 0, that made
it work with TestDriven.Net in previous versions.

On 28 May 2015 at 13:07, Enrico Sada notifications@github.com wrote:

@jcansdale https://github.com/jcansdale see pr #473
#473 .
Disabled both QueryStartUpToDateCheck ( supported = 0 ) and
StartUpToDateCheck.
Now behave like the old code, and testdrive works ok [image: 😄]


Reply to this email directly or view it on GitHub
#471 (comment)
.

@enricosada
Copy link
Contributor

@jcansdale really good bug report! easy to fix.
With your repro was really easy to test it and now testdriven works ok.
Let's wait @latkin or @KevinRansom review of #473

@latkin
Copy link
Contributor

latkin commented May 29, 2015

I investigated a bit - root cause is that TestDriven is calling from off of the main UI thread. Various VS platform APIs will fail with E_INVALIDARG (translates to ArgumentException) if called from off of the main UI thread, and that's what's happening.

I can make a small change so that our up-to-date check avoids such APIs, and we successfully handle the call from TestDriven. Will send a PR soon for this.

However I notice that TestDriven is encountering many other ArgumentException errors for the same reason, unrelated to the up-to-date check. Screenshot below of one example.

I wonder if this is expected? Everything appears to be working (exceptions are handled somewhere), but seems odd.

image

@jcansdale
Copy link
Author

However I notice that TestDriven is encountering many other ArgumentException errors for the same reason, unrelated to the up-to-date check. Screenshot below of one example.

I wonder if this is expected? Everything appears to be working (exceptions are handled somewhere), but seems odd.

There are a few instances where I need to add something to a
collection if it doesn't exist already. Iterating over the collection
can be very slow and there is no method check for an item in the
collection. The least-worst solution it to select the item by name and
handle the exception if the item doesn't exist. This is what's
happening:

try
{
outputWindowPane = outputWindowPanes.Item(name);
}
catch (ArgumentException)
{
outputWindowPane = outputWindowPanes.Add(name);
}

AFAIK there aren't any exceptions being thrown for UI thread related issues.

When integrating directly with the Visual Studio COM objects (without
access to any Windows forms objects), I wouldn't even know how to
marshal onto the UI thread. Is this possible if all I have is the HWnd
of the main Visual Studio window?

Thanks,
Jamie.

On 29 May 2015 at 22:46, Lincoln Atkinson notifications@github.com wrote:

I investigated a bit - root cause is that TestDriven is calling from off of the main UI thread. Various VS platform APIs will fail with E_INVALIDARG (translates to ArgumentException) if called from off of the main UI thread, and that's what's happening.

I can make a small change so that our up-to-date check avoids such APIs, and we successfully handle the call from TestDriven. Will send a PR soon for this.

However I notice that TestDriven is encountering many other ArgumentException errors for the same reason, unrelated to the up-to-date check. Screenshot below of one example.

I wonder if this is expected? Everything appears to be working (exceptions are handled somewhere), but seems odd.


Reply to this email directly or view it on GitHub.

latkin added a commit to latkin/visualfsharp that referenced this issue Jun 1, 2015
…GetActiveConfigurationAndPlatform

 - Give proper consideration to the options flags passed to QueryStartUpToDateCheck
 - Use IVsSolutionBuildManager5.FindActiveProjectCfgName in TryGetActiveConfigurationAndPlatform, as it is not sensitive to sync context
   - Avoids exception for non-UI thread callers, e.g. DTEE automation

Fixes dotnet#471
@latkin
Copy link
Contributor

latkin commented Jun 1, 2015

@jcansdale got it, I misinterpreted the cause of the other exceptions then.

I've opened a PR with a fix for this, will try to get it approved for RTM.

@latkin latkin closed this as completed in 322d27f Jun 3, 2015
@latkin latkin added the fixed label Jun 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants