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

New project system & Cargo based build system #234

Merged
merged 33 commits into from
Oct 21, 2016

Conversation

Boddlnagg
Copy link
Contributor

@Boddlnagg Boddlnagg commented Oct 12, 2016

The Good:

The Bad:

  • Support for VS2013 is dropped, because the new project system doesn't support it
  • Many old tests have been removed, and I have not yet written new tests (but integration tests were disabled on CI anyway and should probably be redesigned to work on AppVeyor)
  • Error markers (squiggly lines) are currently not shown. I haven't been able to figure out why.
  • The parsing of compiler messages is based on Add --message-format flag. rust-lang/cargo#3000 and requires a recent nightly, which will hit stable in December (but this is the only way that cargo build output which includes dependencies can be reliably captured and parsed)
  • .rsproj projects created with previous versions of VisualRust are completely incompatible with this new version

The Rest:

  • Updated to require .NET 4.6 (allows us to use C# 6 features and was necessary because RTVS uses it)
  • Updated most projects to use project.json for NuGet packages (easier to maintain)

The problem was System.Collections.Immutable 1.2.0
Copied from IntelliJ-Rust
They are still referenced from the VisualRust.Build project and copied to it's output directory.
@downtime0
Copy link

downtime0 commented Oct 12, 2016

This is a great change for the project and outweighs the loss of VS2013 support and with VS2015 Community freely available this shouldn't be much of a problem beyond migration.

Great job to those involved.

@Boddlnagg
Copy link
Contributor Author

@vosen: It would be nice if you could comment what you think about this.

Otherwise, if no one has any objections, I'm going to merge in a few days.

Having both VisualRust and RTVS installed led to an MEF composition error (it still does, but loads anyway).
I had previously tried to fix this by renaming IFileSystemMirroringProjectTemporaryItems, but that was
not working. Removing the import of IProjectItemDependencyProvider (which we don't use) is the right fix.
@Vbif
Copy link
Contributor

Vbif commented Oct 15, 2016

Is visit_toml.dll and all Manifest stuff from Cargo project still needed?

@Boddlnagg
Copy link
Contributor Author

Boddlnagg commented Oct 15, 2016

It's still included but currently unused. What's implemented in the PR is only reading the manifest using cargo metadata. As soon as we want to edit the manifest from the UI, we will need some way to parse and edit the TOML file directly, without reformatting everything (i.e. preserving comments and whitespace, etc). That's what vist_toml.dll is built for, but it's unfinished and not really documented. We need to decide what will be done there, but it's out of scope for this PR.

This was referenced Oct 16, 2016
Added a DummyLogger implementation for the now required IActionLog parameter.
This is required to prevent MEF composition errors when RTVS is also loaded.
@vosen
Copy link
Member

vosen commented Nov 7, 2016

The idea of moving us to CPS is sound and is something that I wanted to do.
The problem was always that would have required a lot of effort to port: redoing stuff with MEF and .xaml rules is time consuming and even things like .pkgdef are extremely fiddly.
This commit pretty forces my hand in this regard.
While I'm not 100% enthusiast of removing tests and I see that some thing are broken (.rsproj loading, probably due to aforementioned pkgdef, tested on a clean machine) I'm Ok with the commit. Someone had to do it, sooner or later.

@Boddlnagg Boddlnagg mentioned this pull request Feb 21, 2017
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider moving to VS Common Project System
4 participants