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

[build] use WorkingDirectory for <GitBlame/> #2594

Merged
merged 1 commit into from Jan 10, 2019

Conversation

Projects
None yet
2 participants
@jonathanpeppers
Copy link
Contributor

jonathanpeppers commented Jan 8, 2019

Downstream in monodroid, I'm getting a build failure such as:

/usr/bin/git blame "/full/path/to/external/xamarin-android/Configuration.props"
XAVersionInfo.targets(53,5): error MSB6006: "git" exited with code 128.
XAVersionInfo.targets(61,5): error MSB4044: The "GitCommitsInRange" task was not given a value for the required parameter "StartCommit".

If I run the git command manually:

fatal: not a git repository (or any of the parent directories): .git

It seems we shouldn't be using a full path here, but use
WorkingDirectory instead? Maybe it will not work if
xamarin-android is a submodule?

This has somehow been working in the past, so perhaps my version of
git is the cause?

$ git --version
git version 2.17.1 (Apple Git-112)
[build] use WorkingDirectory for <GitBlame/>
Downstream in monodroid, I'm getting a build failure such as:

    /usr/bin/git blame "/full/path/to/external/xamarin-android/Configuration.props"
    XAVersionInfo.targets(53,5): error MSB6006: "git" exited with code 128.
    XAVersionInfo.targets(61,5): error MSB4044: The "GitCommitsInRange" task was not given a value for the required parameter "StartCommit".

If I run the git command manually:

    fatal: not a git repository (or any of the parent directories): .git

It seems we shouldn't be using a full path here, but use
`WorkingDirectory` instead? Maybe it will not work if
`xamarin-android` is a submodule?

This has somehow been working in the past, so perhaps my version of
git is the cause?

    $ git --version
    git version 2.17.1 (Apple Git-112)

@jonathanpeppers jonathanpeppers requested a review from jonpryor Jan 8, 2019

@jonathanpeppers

This comment has been minimized.

Copy link
Contributor

jonathanpeppers commented Jan 8, 2019

Oh apparently someone else had this problem: #2574

@jonathanpeppers

This comment has been minimized.

Copy link
Contributor

jonathanpeppers commented Jan 8, 2019

Don't know what happened to the build...

06:15:25 (_BuildAndroidSqlite target) -> 
06:15:25   /Users/builder/jenkins/workspace/xamarin-android-pr-builder-release/xamarin-android/external/sqlite/dist/sqlite3.c(33031,9): error G3127DA5A: use of undeclared identifier 'ANDROID_FDSAN_OWNER_TYPE_SQLITE'; did you mean 'ANDROID_FDSAN_OWNER_TYPE_FILE'? [/Users/builder/jenkins/workspace/xamarin-android-pr-builder-release/xamarin-android/src/sqlite-xamarin/sqlite-xamarin.csproj]

I got the same error when I rebased: #2590

@jonpryor

This comment has been minimized.

Copy link
Contributor

jonpryor commented Jan 9, 2019

The current build failed because the machine had NDK r18 installed (because that build machine previously built PR #2592), and thus the build machine is now "corrupt". :-(

We need to figure out support for NDK downgrades.

@jonathanpeppers

This comment has been minimized.

Copy link
Contributor

jonathanpeppers commented Jan 9, 2019

build

@jonpryor

This comment has been minimized.

Copy link
Contributor

jonpryor commented Jan 9, 2019

It seems we shouldn't be using a full path here, but use WorkingDirectory instead?

The problem isn't the full path (and we want the full path, just for sanity).

The problem is that your git checkout is (partially) FUBAR.

Unfortunately I can't readily find how to fix a FUBAR repo, short of nuking it and checking it out again.

For example, a few weeks ago I moved my git checkout from one directory into another. git was not happy with this decision, because sometimes in git submodules the .git file contains a full path instead of a relative path, e.g.

$ cat external/mono/external/nuget-buildtasks/.git
gitdir: /Volumes/Xamarin-Work/xamarin-android/.git/modules/external/mono/modules/external/nuget-buildtasks

That's the new dir, not the old dir, but the problem is that it's a full path, so when I moved my git checkout, the path was invalidated, and every git operation which attempted to deal with that directory (and the 10 others that also had a full path) would fail, all of them saying that it wasn't a git repository.

I had to find and manually edit each of these in order to get git working again.


Regardless, using a full path should be perfectly valid, and if git fails on that, there's not much we can do.

What we can do is improve the error message. What we currently generate is not helpful, because the actual git error message is buried or not otherwise printed, which is why you had to manually run the command to see what the error was.

We should fix that: surface the fatal: not a git repository message so that the build log has something useful to go on.

@jonathanpeppers

This comment has been minimized.

Copy link
Contributor

jonathanpeppers commented Jan 10, 2019

So I did a fresh checkout and I'm still having the problem with our private repo:

git clone https://github.com/xamarin/our-private-repo.git somedir
cd somedir
git submodule update --init --recursive
./configure --build-windows-cross=false
make reset-versions

This failure:

$ /usr/bin/git blame "/Users/myuser/Desktop/git/somedir/external/xamarin-android/Configuration.props"
fatal: '/Users/mysuser/Desktop/git/monodroid2/somedir/xamarin-android/Configuration.props' is outside repository

But this command works fine:

cd /Users/myuser/Desktop/git/somedir/external/xamarin-android/
/usr/bin/git blame "Configuration.props"

That's why I'm wondering if we can set WorkingDirectory to the full path, then use a related path within the repo's directory.

@jonpryor

This comment has been minimized.

Copy link
Contributor

jonpryor commented Jan 10, 2019

git requires that $CWD be within a git checkout, so this will fail:

$ cd $HOME
$ git blame /path/to/Configuration.props

(Unless $HOME is a git checkout, but how many people do that?)

When within a git repo, using a full path Works For Me™

$ cd /path/to/xamarin-android
$ git blame /path/to/xamarin-android/Configuration.props
# no error

Discussing this on gitter, it could be a git "bug"/"feature". It works for me with Apple git 2.17.2, while fails for @jonathanpeppers using Apple git 2.17.1.

Which suggests that we should accept this PR, to decrease git version dependencies.

@jonpryor

This comment has been minimized.

Copy link
Contributor

jonpryor commented Jan 10, 2019

build

@jonpryor jonpryor merged commit 3576678 into xamarin:master Jan 10, 2019

3 checks passed

Ubuntu PR Build Build finished. No test results found.
Details
license/cla All CLA requirements met.
Details
macOS PR Release Build Build finished. 172335 tests run, 39 skipped, 0 failed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment