Skip to content
This repository has been archived by the owner on Feb 12, 2023. It is now read-only.

Simpler, more automatic GitLink #110

Merged
merged 74 commits into from
Jan 18, 2017
Merged

Conversation

AArnott
Copy link
Contributor

@AArnott AArnott commented Oct 9, 2016

This PR is a significant refactoring of both code and how functionality is exposed. This includes:

  1. Installing the GitLink nuget package into a project automatically activates GitLink in that project's build. The README instructions are updated to reflect just how easy it is now compared to before.
  2. gitlink.exe is now focused on the PDB instead of Visual Studio solutions. The only argument required is the PDB file. Everything else is automatic.
  3. pdb file information always has the canonical capitalization expected by the web server.
  4. added support for PDBs that carry SHA1 hashes instead of MD5. I don't know who still uses MD5, but csc.exe seems to always produce SHA1 hashes.
  5. packages.config have been migrated to project.json, resulting in cleaner, simpler build authoring that's easier to manage.
  6. packages and assemblies versioned with git information.
  7. NuGet package builds with the same MSBuild invocation that builds the solution. And if the NuProj extension for VS is installed, it builds within VS as well.
  8. Builds on appveyor. Once this PR is accepted, appveyor should be added to this repo and the README badge can be updated to point to your appveyor build instead of mine.
  9. Print out how many files ended up getting indexed into the PDB, to make noticing issues easier.
  10. Tests are upgraded to NUnit 3, and the NUnit test adapter for VS is added so that tests automatically run in VS even for folks who haven't installed the NUnit extension.
  11. Recognize and report failure even when pdbstr.exe fails with a 0 exit code.
  12. README file is reordered to emphasize benefits and usage instructions over troubleshooting and how it works internally.
  13. GitLink NuPkg is ready for chocolatey. That is, the same package for nuget.org also works on chocolatey directly.
  14. nupkg license points at the exact license that produced the package rather than the tip of 'develop', which may have diverged from what the package was built with. (mostly theoretical, but sound).

Noteworthy other changes

  1. The command line tool no longer accepts a 'solution directory', configuration/platform, or anything else MSBuild-related.
  2. The project and NuGet package are currently optimized for consumption rather than extensibility. This is based on the belief that most people who are interested in GitLink want to use it on their projects to benefit their users. If the refactored form of GitLink is not applicable enough for some people's scenarios that they still want a way to consume it programmatically, we can revisit this decision.
  3. The GitLinkTask nuget package is removed. It's all about the GitLink package now.
  4. gitlink.exe doesn't contain all its dependencies as embedded resources. I consider this a feature. It builds with a much more recognizable toolset now, and can be invoked many times in a row with higher performance due to all files being ready for use instead of having to be extracted for every invocation.
  5. Notice the count of added and removed lines in this diff. This makes the project overall significantly smaller, yet more adaptable and easy to use. :)
  6. Migrate to use StyleCop.Analyzers instead of StyleCop.

Remaining work

  • Write new unit tests to cover the new entrypoints. No new test holes were created in this PR, but I think we can improve our test coverage here.
  • Use GitVersion
  • Keep the output folders as-is (we can discuss the use of read only source tree later)
  • Keep SolutionAssemblyInfo (we can discuss this later, let's focus on the actual issue we are trying to solve here 1 at a time)
  • Keep the batch files for now (they aren't sitting "in the way" for features to be added)
  • Fix perf issues with case normalization
  • Merge Implementation of UNC provider with support for forward and back slas… #132

Closes #104

Use project.json instead of packages.config
Stop linking assemblies together and embedding executables.
Also replace cmdline parsing
Rip out MSBuild-oriented Linker method
Now the pdb.srcsrv file actually works. The debugger can download files.
Command line switches can provide all the necessary information.
@GeertvanHorrik
Copy link
Contributor

I added an additional task to merge #132 once that is completed.

@GeertvanHorrik
Copy link
Contributor

  1. Yes, let's skip (additional) unit tests for now
  2. @gep13 interested in helping out here?
  3. This affects a wider discussion since we want all GitTools repositories to be the same. I prefer to keep this out of scope for this release and discuss whether we need to apply the read only source tree for all repositories.
  4. Let's leave it as-is for now, we can bring it back if we really miss it

Sorry for my late replies, but the holidays are coming up so deadlines are popping up everywhere.

@AArnott
Copy link
Contributor Author

AArnott commented Dec 20, 2016

No problem. Thanks. I'll work on removing the read only source tree. If I've done my work well, hopefully it is as simple as removing the nuget package.

@AArnott
Copy link
Contributor Author

AArnott commented Dec 20, 2016

OK, my develop branch no longer has the readonlysourcetree package. Removing that dependency was the only change I needed to make, plus to appveyor.yml so that it would still find the nupkg to package as an artifact.

I've modified from their original to be more reliable to paths with spaces, require at least MSBuild 14 (because that's the only thing tested), and clean build outputs from the new location
@AArnott
Copy link
Contributor Author

AArnott commented Dec 22, 2016

I've brought back the build scripts.
Can you tell me more about PR #132 and why that needs to be merged, and whether you intend it to be merged into my develop branch before merging into yours?

@AArnott AArnott reopened this Dec 22, 2016
@GeertvanHorrik
Copy link
Contributor

It's support for a new provider (UNC paths). I will merge it now.

@AArnott
Copy link
Contributor Author

AArnott commented Dec 22, 2016

OK, then I guess my next task will be to resolve merge conflicts.

@AArnott
Copy link
Contributor Author

AArnott commented Dec 24, 2016

Merge conflicts resolved. It's just GitVersion holding the merge back now, I think.

@GeertvanHorrik
Copy link
Contributor

@gep13 are you on holidays? Otherwise I will pick the GitVersion thing up in the first week of January.

@gep13
Copy link
Member

gep13 commented Dec 24, 2016

@GeertvanHorrik I am on holiday now, yes (i.e. have some time off work) however, I am full of the cold/flu, so won't be looking at this for at least a couple days. I will let you know if I get a chance to have a look.

@JakeGinnivan
Copy link

I just had a quick scan, what is the GitVersion issue holding this back?

@gep13
Copy link
Member

gep13 commented Dec 28, 2016

@JakeGinnivan said...
I just had a quick scan, what is the GitVersion issue holding this back?

I don't think that there is anything holding it back, it just needs to get done. I haven't had the cycles to sit down and add it into the build process yet.

@GeertvanHorrik
Copy link
Contributor

@JakeGinnivan nothing in GV is holding this back, what @AArnott meant was that we need to apply GV to GitLink (on this specific PR), hence the "GV issue". We will just use the latest stable version for now.

@GeertvanHorrik
Copy link
Contributor

@AArnott this is fully done except for the GitVersion stuff? Then I will release a new stable version of GL and merge this into develop.

@GeertvanHorrik
Copy link
Contributor

GitLink 2.4.0 has been released, we are ready to merge this if @AArnott gives the go ahead. This will become 3.*. I will update GV on the develop branch once merged.

@AArnott
Copy link
Contributor Author

AArnott commented Jan 17, 2017

Yes, this is ready. Thanks.

@AArnott
Copy link
Contributor Author

AArnott commented Jan 17, 2017

Sorry it took so long for me to reply with the go ahead.

@GeertvanHorrik GeertvanHorrik merged commit d70d530 into GitTools:develop Jan 18, 2017
@GeertvanHorrik
Copy link
Contributor

Merged, this is a major step forwards. Thanks for all the effort @AArnott . We will now slowly "fix" all the missing pieces and start doing prereleases.

@AArnott
Copy link
Contributor Author

AArnott commented Jan 18, 2017

Awesome.

@kzu
Copy link

kzu commented Jan 30, 2017

@AArnott does this mean that from now on PdbGit will be "obsolete"?

@AArnott
Copy link
Contributor Author

AArnott commented Jan 31, 2017

@kzu Yes, once GitLink 3.x ships. I'm assuming that GitLink 3.0 retains the characteristic that I added in this PR that when you install it to an MSBuild project, it immediately "just works". Because to me that is very important. No build authoring should be required to activate this.

@kzu
Copy link

kzu commented Jan 31, 2017

Agreed. The CI build is currently broken , unfortunately :(

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants