dotnet/docfx

Getting branch name fails on AppVeyor

richardschneider opened this issue · 15 comments

richardschneider commented

Title

Getting the branch name fails on AppVeyor

Functional impact

Seeing this error:

Info: Can't find remote branch in this repo and fallback to use local branch [HEAD]: fatal: HEAD does not point to a branch

The View source link is wrong. It is using HEAD instead of the branch that is being built.

Minimal repro steps

I've cloned docfx-seed at https://github.com/richardschneider/docfx-seed and created a branch named msdn-xref (see issue #1078).

Further technical details

Most CIs, such as AppVeyor, use a git detached head to build a new commit.

richardschneider commented

GitUtility.GetRepoInfoCore needs to take a detach head into consideration. Most CIs create an environment variable with the current branch name

qinezh commented

Just like my comments in your PR, I don't think it's a bug in DocFX.

cameronelliott commented

I will leave my thoughts on this too, in case it is helpful.
I have run into the same problem using AppVeyor.

Richard> You might find a 'git checkout master' helps you, even if it is not a nice/clean solution.

Qinezh>
I don't personally like how Docfx trys scanning the file-system and git-repo in order to determine whether "Improve this document" links should be generated.
I wonder if "Microsoft.DocAsCode.Common/Git/*" could be removed, and
the flags _gitContribute, _gitUrlPattern, and maybe another new flag could be used to specify git-repo linking without Docfx having to be able to read Git repo files, etc.
Just a crazy idea. :)

qinezh commented

I submit a PR #1100 to fix the wrong branch name when doc repo in detached head. In this case, we'll use commit id instead of HEAD as the branch name.

richardschneider commented

PR #1100 is a good fall back. However, it does help with building documentation on AppVeyor.

When building the doc, I want the yml to point to the branch, not the commit id. Basically, use doxfx to create the documentation and then push to gh-pages. By using a commit id, every entry in gh-pages will be changed on each build and my repo gets very big.

qinezh commented

So you want to build documentation from history commit but generate view source url with branch name instead of commit id, am I right?

richardschneider commented
qinezh commented

In this case, why not use git checkout <branch_name> once AppVeyor has cloned your repo with detached head?

richardschneider commented
qinezh commented

OK, let's assume that you're building a PR in AppVeyor, which branch you want to show in view source url, or in another word, what's your expected view source url?

cameronelliott commented

I had the same issues Richard had, I am not saying there is a 'correct' solution, but my recommendation
would be for the team leaders to have a discussion about the following:

Team leader discussion: @qinezh @vicancy @vwxyzh

  1. "Should we remove the git repo filesystem reading code, and just require the user to configure the git URL mapping inside of docfx.json?"

If this can work, you are the experts on whether it is a good idea or not,
but it might have these benefits:
[I am sorry I do not speak Mandarin or Cantonese]

  1. No git libraries in a documentation creation tool. [docfx]
  2. Follows design principal "Principle of least astonishment". https://goo.gl/mkdQhK
    [I was really really surprised docfx scanned my repo to implement "Improve this doc", and it took a lot of work to figure out how to disable it, or get it to work on Appveyor. I was surprised I could not fully configure this behavior solely using docfx.json]
    [I actually downloaded DocFx source code to step through it with the debugger to work around the issue. Remember, your end-users [me & Richard] do not know about your design decisions, and we must learn them.
    And it is slow and hard for us to understand how things work, not because we are stupid, but because
    this is the path for everybody to learn the design characteristics of someone else's project.]
    [I was very suprised this was not fully configured via docfx.json]

It is of course, your team's decision, but I know outside user-experience feedback can be helpful, so I am sharing my experience and thoughts.

I really really like DocFX, by the way!

I use it for a project I am working on: https://cameron-elliott.github.io/LimeVideoSDK/
My documentation gets built on each github commit using Appveyor.
https://github.com/cameron-elliott/LimeVideoSDK/blob/master/appveyor.yml

But honestly I had to work around a number of surprising difficulties to get it to work correctly.
I hope by providing feedback, it can become easier to use docfx+Appveyor for future users.

qinezh commented

@richardschneider

I have some solutions for your problem:

Option 1: in AppVeyor part:

  • For normal case, git checkout <branch_name> should be ok.
  • For PR, you could ignore the step to run git checkout <branch_name> with AppVeyor Environment variable, such as APPVEYOR_PULL_REQUEST_NUMBER. Or you could just ignore the step to run DocFX for PR if you don't need do something like E2E Test...

Option 2: in DocFX part:

Then let's talk about your PR. I think there're some hard logic in it, which may cause other issue, such as, "Why I can't generate view source url with specific commit id in AppVeyor?"

@Cameron-elliott

We have a plan to add a switcher to disable git command for uses who don't need git information.

As for your advice, I don't think it's a good solution to require users to configure git URL mapping in docfx.json, since some users may want DocFX to get git information automatically.

Then, how about adding git URL mapping as an option? Honestly speaking, I couldn't find common scenario to use this config. Of course, maybe you want to use the config while git command is disabled with the switcher I mentioned. But I don't think it's a common case and why not use git command directly...

@richardschneider @Cameron-elliott I just want to help resolve this issue, please don't be mind :)

richardschneider commented

@qinezh Yes, Option 1 would work for me (most of the time). But PR #1098 fixes the problem for me and for anyone else using docfx+appveryor.

If you have issues with the PR, please elobarate on the PR.

DerAlbertCom commented

We use this as a Powershell Script on VSTS to get the right branches.

git checkout master
git submodule foreach 'git checkout master'

We have submodules for different repositories in the /src folder.

But as PowerShell ist critical, because as .bat the checkout of the submodules does not work because of missing git ....

richardschneider commented

PR #1098 fixes this issue.