TINKERPOP-1836 Add Gremlin.Net.Template project

[FlorianHockmann]

https://issues.apache.org/jira/browse/TINKERPOP-1836

This adds a dotnet template that can be installed and then used to create a simple dotnet console application ready to be used together with a Gremlin Server.

We need to release the template just like Gremlin.Net itself as a NuGet package to nuget.org.

IMPORTANT: The Maven package now requires Mono when .NET should be built as it's not possible to pack dotnet templates without Nuget (dotnet pack doesn't work for this unfortunately).

./docker/build.sh -t -i ran successfully until it failed due to some Giraph tests and the .NET projects were already built successfully. Since the Giraph tests are known to be flaky and we already deprecated Giraph, I didn't look further into that.

VOTE +1

[spmallette]

Cool. A few comments/questions:

1. sorry - you will need to rebase as i just updated dev docs and now this PR is conflicted
2. I guess the Giraph failure doesn't matter - it seems like this change really couldn't have triggered that failure
3. I feel like it would be good to add some user docs for the template, no? I do see that you have the README, but I think having something in those reference docs would be good to as that's easy to link to. Maybe add to this section: http://tinkerpop.apache.org/docs/3.2.9/reference/#gremlin-archetypes (probably should just rename that to "Application Templates" (leave the "gremlin-archetypes" anchor though)?
4. regarding " The Maven package now requires Mono when .NET should be built as it's not possible to pack dotnet templates without Nuget (dotnet pack doesn't work for this unfortunately)." do you just mean that the dotnet-maven-plugin call to pack doesn't work ( https://github.com/apache/tinkerpop/pull/867/files#diff-eb2d4cdb0a821b1a1e7166f54bc46d2aR87 ) for templates?

[jorgebay]

Maybe we could wait until dotnet toolchain supports packaging and pushing templates (is there a ticket for it?), instead of increasing the complexity of the deploy process by requiring Mono.

[FlorianHockmann]

Rebased and I extended the gremlin-archetypes section in the reference docs by splitting it into two sections: Maven Archetypes and Gremlin.Net Template. In the future, we can add more sections when the other GLVs also get templates. gremlin-archetypes is now the anchor of the Maven archetypes subsection.
However for some reason the generated docs have an added comment in the last listing and I don't know how I can prevent that from happening. It looks like this:

dotnet new gremlin -o MyFirstGremlinProject
// LAST LINE

do you just mean that the dotnet-maven-plugin call to pack doesn't work

Unfortunately not, it simply doesn't work with the dotnet CLI tool. So there really isn't anything the plugin can do to help us here. Here is what the dotnet docs say about that:

Currently, a custom template is packed on Windows with nuget.exe (not dotnet pack). For cross-platform packaging, consider using NuGetizer 3000.

I never used (or heard about) this NuGetizer and it looks a bit outdated. So I'm sceptical whether that could help us.

Maybe we could wait until dotnet toolchain supports packaging and pushing templates (is there a ticket for it?), instead of increasing the complexity of the deploy process by requiring Mono.

Unfortunately, I don't think that the dotnet CLI tool will support that any time soon. The ticket in question should be NuGet/Home#4254. It has 20x 👍, was created over a year ago, but it doesn't look as if they wanted to address this issue in the near future.

Nevertheless I share your concern that we shouldn't make the build too complicated, especially for new contributors who just want to submit a PR. Shouldn't it be possible to build the .NET projects from Maven and execute their tests without having to execute the package phase? I just tried it with mvn clean test, but that didn't execute any tests to my surprise.
You mentioned that you don't want to increase the complexity of the deploy process, but that already required Mono even without this PR. Or at least that's what the dev docs currently say:

For those who will release TinkerPop, it is also necessary to install Mono.

When we decide that we don't want to add this template right now, then I could also host it from my own GitHub until dotnet pack works with templates. The downside with that approach is of course that it would be harder to find the template, especially for new users who need it in the first place.

[spmallette]

@FlorianHockmann i don't know if any of the following makes any sense, but would it be crazy to only package templates on deploy (since mono is already required there)? or do we need to continually build it because it could break or fail tests (i.e. maven archetypes are built and also tested for problems on every build)? Or, would it be crazy to only package templates if mono is present? Or perhaps it is somehow otherwise an optional aspect of the build (except on deploy)?

[FlorianHockmann]

Or, would it be crazy to only package templates if mono is present?

I think that would be the best solution here. It wouldn't introduce any new dependencies to the build and it would still allow packaging of the template locally to test it just like the version that gets deployed (for contributors who have Mono installed but that's a necessity with every solution).

[FlorianHockmann]

Turns out that it was very easy to add a check if mono is present and otherwise skip the package of the template:

<if>
    <available file="mono" filepath="${env.PATH}"/>

I rebased and squashed the commits as I had to revert changes I made to the dev docs because mono now isn't required any more to build the gremlin-dotnet module.

[spmallette]

Your approach with mono works pretty nicely. It still bothers me a little bit that every build though requires download of nuget though for just a mvn package. That of course means that you can't build without an internet connection:

main:
     [exec] --2018-05-21 10:32:16--  https://dist.nuget.org/win-x86-commandline/v4.4.1/nuget.exe
     [exec] Resolving dist.nuget.org (dist.nuget.org)... failed: Name or service not known.
     [exec] wget: unable to resolve host address ‘dist.nuget.org’

I suppose you could hit maven failures for "no internet" if you don't have all your .m2 stuff cached, but at least its possible to get a build without it. Going to think about this some more....maybe not a big deal i suppose.

[FlorianHockmann]

You're right. It shouldn't be necessary to have an internet connection when all dependencies are already available. I thought that nuget.exe would only be downloaded once as we have the available file checks, but unfortunately nuget.exe was downloaded to a bin folder which gets cleaned with mvn clean. So I just pushed a commit that downloads nuget.exe instead directly into the gremlin-dotnet directory and now it won't be downloaded again after that.

[spmallette]

I guess that will work....I wonder if this should be a bit more bulletproof though? on deploy phase we used to get a fresh nuget every time so if the version changed from 4..4.1 we were in good shape. Now we're forced to manually delete because we clean doesn't really cleanup nuget.exe. Could we rename nuget.exe to nuget-441.exe on download? then if we changed version for some reason, you'd get the right version.

maybe make the nuget-*.exe and version maven <properties> of gremlin-dotnet-source so that we can edit them centrally as well?

[FlorianHockmann]

so if the version changed from 4..4.1

Good point, I haven't considered that. I amended my last commit to include the solution you proposed. The downloaded exe now simply contains the version number (like nuget-4.4.1.exe).

[jorgebay]

Would it be possible to move the execution to the deploy phase? that way on install or prior phases, the build wouldn't be affected.

Also, it won't require mono on TravisCI or docker-based builds.

[spmallette]

i'd asked that question earlier

#867 (comment)

@FlorianHockmann response follows that.

[jorgebay]

I would like to propose that we generate and build Gremlin.Net.Template project and run its tests as part of the package phase but we require nuget (to run nuget pack) only as part of the deploy phase.

That way we don't affect developers / collaborators that want to use mvn clean install and run the tests. Also, this way we avoid taking additional time on CI builds.

[spmallette]

I tend to agree with @jorgebay unless there is some high risk of something breaking in the template packaging (as i understand it we need nuget so that we can do nuget pack) on release day when we actually execute the deploy phase. If it fails on release day that would be really bad because we'd already have a positive closed VOTE thread and we would already be half of the other artifacts would have already been deployed.

If @FlorianHockmann can explain why there is sufficient risk for failure during nuget pack perhaps we can agree to make that process a documented "Pre-flight Check" step of the release process that is triggered manually with mvn clean install -Dnuget. That way we can test it ahead of VOTE and not have it be part of the regular build process.

[FlorianHockmann]

we require nuget (to run nuget pack) only as part of the deploy phase.

I understand that part and I don't have anything against it, especially when we add something to the "Pre-flight Check" step as @spmallette suggests.

But @jorgebay, what's your reasoning behind

I would like to propose that we generate and build Gremlin.Net.Template project and run its tests as part of the package phase

?

Shouldn't the template be built and tested in the same phases as Gremlin.Net itself?

[jorgebay]

Shouldn't the template be built and tested in the same phases as Gremlin.Net itself?

Yes, I probably didn't make it clear :) I wanted to say any maven phase for generating and building.

[FlorianHockmann]

I just rebased and made the changes so that now the template is only packed with:
mvn clean install -Dnuget.

[jorgebay]

Looks great! I've included a comment above, but in any case: VOTE +1

[jorgebay]

I think we can remove mono sdk from travis.

[FlorianHockmann]

Done

[spmallette]

VOTE +1 - thanks again @FlorianHockmann for dealing with another PR that the required a lot of back/forth review

[FlorianHockmann]

Rebased again to fix a merge conflict.