https://github.com/palantir/godel/wiki/Configuration is missing generate.yml section.

init.sh reload is an optional command for sending a SIGHUP to the application. Unfortunately if you don't handle the SIGHUP, your process dies.

There should be reload: true option for distgo which includes the reload command. This will eliminate a reason that applications need to use templates.

Currently check novendor will flag dependencies that are required by code run as part of generate tasks. It would be nice if these vendored packages could be accounted for as part of the novendor and extimport to make it possible to vendor generation libraries.

Currently, the os-archs field is required for os-arch-bin distributions. However, it makes sense to have the behavior mirror what is done for build and have it default to using the OS/Arch combination at runtime if it is unspecified.

Currently, command-line arguments cannot be passed to the run command. Should make it so that the trailing arguments are passed to the program being run.

Docs directory/wiki docs has instructions on how to install, but without prior context it's hard to know how to actually set up a project and do things like run tests, configure distribution, etc. Would be nice to add a tutorial to walk through this.

1. Create a directory in the project directory that cannot be read by the current user: sudo mkdir foobar; sudo chmod 0700 foobar
2. Add foobar as an excluded path to configuration
3. Run ./godelw test

Expected

Tests should run properly without any errors

Actual

The tests do not run and the command fails with an error like the following:

failed to list packages in /.../foobar: open /.../foobar: permission denied

If a path in the project is excluded by configuration, then its existence should not cause a failure even if the user running the command cannot read it.

In some instances, a Go project may contain output artifacts that are meant to be generated and published based on the Go code in the project, but do not involve creating artifacts based on Go files in a traditional manner.

An example of this is a Go project that generates an AWS lambda from a Go package using apex build. Such a project may want to use godel for running checks and managing other aspects of the build and for publishing the generated artifacts while delegating the output generation itself to another task/tool.

Support for this will be added in the following manner:

• Add a new skip parameter to the build configuration. When this parameter is "true", the build step for the product is always skipped.
• Add a new manual dist type. This dist type will allow an extension (such as tgz or zip) to be specified. If the extension is unspecified, it is assumed to be empty.
• The manual dist type assumes that an output file of the form {{product}}-{{version}}.{{extension}} (or {{product}}-{{version}}) will exist in the dist output directory.
  • Generally, the script property of the dist should include a script that produces output in this location. Within the dist script, the output location can be represented as `$DIST_DIR/$PRODUCT-$VERSION.[extension]" (the extension is not available as an environment variable and should be specified in the script).

The above makes it possible to specify a "dist-only" product that does nothing on build, but on dist will create a dist output using the script and on publish will publish the created artifact.

We had a build fail recently on the almanac publish step. The error message leaks the Authorization header which includes the almanac credentials. E.g:

Almanac publish failed for sls: failed to create product sls: Almanac request failed: {POST https://almanac.palantir.com/v1/units/products  0 0 map[X-Authorization:[X...REDACTED...6:9...REDACTED...=] Content-Type:[application/json] X-Timestamp:[1481486955]] {0xc420b261e0} 14 [] false  map[] map[] <nil> map[]   <nil> <nil> <nil> <nil>}: Post https://almanac.palantir.com/v1/units/products: x509: certificate signed by unknown authority

There are many workflows where a user will have many different modules in a single project directory, each with possibly a different version of godel. In that case, each module should use its own version rather than a shared project version.

To fix this, the invoked program should be changed from $ProjectFileDir$/godelw to $ModuleFileDir$/godelw

The curl call in the godelw script to download the distribution currently does not follow redirects, so if the distribution URL requires a redirect the download will fail.

The configuration of the command should be updated to follow redirects.

A la https://github.com/palantir/gradle-docker

Since distgo already enforces building RPMs only for linux-amd64, it should probably pass -a amd64 and --rpm-os linux to fpm so the resulting RPM is usable even if built on OS X (currently when running on OS X it builds me an RPM containing linux-amd64 binaries that is only installable on darwin-amd64 systems)

If you run ./godelw test --tags=bla you get a message saying invalid tags: "integration". This is completely reasonable once you know how the configuration of tags works.

However, since go test -tags=bla will work (if you add // +build bla at the top of a test file), this is pretty confusing. Could we consider making the error message more clear, for example by saying something like tag "bla" not found in test.yml or see the documentation for how to add tags in godel at https://github.com/palantir/godel/wiki/Configuration#testyml.

It would be helpful to have the idea task generate "Run" and "Debug" IDEA configurations for the main packages in the project. Could either implement this as generating for all the main packages or just implement it for the products defined in dist.yml

Godel has logic to determine if the build is a tag release or a snapshot release and creates the version string accordingly, but has no way to output it.

I have to do something like this to parse it out ./godelw artifacts dist | cut -d '-' -f 2- | sed 's/.tgz//'.

IMO, ./godelw --version should print godel's version as it does today, but ./godelw version should print the artifact's version, not godel's version. How does that sound @nmiyake ?

The ./godelw check-path currently applies the fixes for the issues that it identifies by default. Although there is a --dry-run flag that allows the behavior to only check without fixing, this isn't super intuitive -- have received feedback that it would be better for --dry-run to be the default behavior.

Likely path forward is to remove --dry-run flag and add an --apply flag instead, and have the default behavior be false for that flag.

I think the argument isn't making it to fpm. I have a workaround for my current use case, but will probably take a stab at fixing this soon.

1. Add the https://github.com/mattn/go-sqlite3 library to a project, but omit the required C files (example of this is adding using govendor update)
2. Run the compiles check

Expected

Output of check should be similar to that produced by go build:

github.com/.../vendor/github.com/mattn/go-sqlite3
In file included from vendor/github.com/mattn/go-sqlite3/backup.go:9:
./sqlite3-binding.h:2:10: fatal error: 'code/sqlite3-binding.h' file not found
#include "code/sqlite3-binding.h"
         ^
1 error generated.

Actual

The check runner fails to parse the output, which results in output like the following:

Running compiles...
check compiles failed
 --- at /home/ubuntu/.go_workspace/src/github.com/palantir/godel/vendor/github.com/palantir/godel/apps/okgo/cmd/check.go:67 (DoRunAll) ---
Caused by: Failed on line 1: line is not valid as the start token for an issue: "In file included from /.../github.com/.../vendor/github.com/mattn/go-sqlite3/backup.go:9:"
 --- at /home/ubuntu/.go_workspace/src/github.com/palantir/godel/vendor/github.com/palantir/godel/apps/okgo/checkoutput/parser.go:61 (ParseIssues) —

Suggested fix

Either:

• Figure out a way to properly parse and print multi-line error output

or

• If it's extremely hard/expensive to process multi-line output, print such output as quoted text so that it condenses to a single line (use \n etc.)

Some projects have conventions around how certain struct tags should be configured. For example, one may want to enforce that all json: struct tags have a value that is in camel case, or that all yaml: struct tags have a vale that is kebab case.

The check should be configurable as follows:

• Specify that tag name to check (json, yaml, etc.)
• Specify the validation that should be performed on the content of the tag
  • camelCase, kebab-case, etc.
• Mechanism for white-listing specific structs in specific packages to exclude from the check (even if a project has a convention, in some cases it may be necessary to violate it for compatibility with other pre-existing data structures)
  • Preferably whitelisting mechanism would be part of code/struct rather than a separate list maintained in configuration, but TBD

Currently, the version used by distgo for a repository uses the output of git describe to get the version. Although this is fine in the common case, it's often not the desired behavior if merge commits exist.

For example, consider the following case:

• Mainline branch has some tag (1.0.0-dev) and development is proceeding normally
• Some other branch (for example, 1.0.0-hotfix) is merged into the mainline branch
• Distribution is created on the mainline branch

In this scenario, we typically want the version to still be 1.0.0-dev-*. However, because git describe returns the nearest ancestor tag, the merge causes 1.0.0-hotfix to be found as a parent tag that is "closer" to the commit than 1.0.0-dev, so you get 1.0.0-hotfix-* instead.

The fix would be to match the behavior of git describe --first-parent instead -- this command only considers the first parent commit for merge commits, and would cause this to work correctly. I believe that this is almost always the desired behavior.

If godelw needs to download the distribution, it determines whether or not wget exists on the system and, if it does, it attempts to download the distribution using it. If the attempt fails, the script then exits with a failure even if curl is also available.

Would be more robust to also attempt the download using curl if it is available on the system. This can be helpful in cases where wget may exist on a system but be misconfigured.

Some of the checks in okgo (such as errcheck, outparamcheck and varcheck) use the https://github.com/kisielk/gotool library, which in turn uses build.Default (https://github.com/golang/go/blob/e646d07329e7edc56c8cf3284c729937979dae4a/src/go/build/build.go#L259) as the build context. The ReleaseTags variable of the context (https://github.com/golang/go/blob/e646d07329e7edc56c8cf3284c729937979dae4a/src/go/build/build.go#L47) are used to determine which files to include as part of the build when running the check.

The issue is that the content of these variables are determined by the version of Go compiled into gödel. Thus, if gödel is compiled using go1.8, then even if it is run in an environment where the version of Go on the system/used by the project is 1.7, the build directives will behave as if the version of Go is 1.8, which can lead to issues (since files protected with a version build tag typically make use of version-specific features). The reverse is also true -- if gödel is compiled using Go 1.7 and run in a Go 1.8 environment, the checks will only include the files matched by the go1.7 tag.

There isn't much that can be done about the forward-compat case (and there shouldn't be too many adverse effects), but the back-compat case is interesting and needs to be handled (or else we need to maintain different branches of gödel per Go version, but would like to avoid that).

Because build.Default and context.ReleaseTags are both exported variables, it is possible to update/re-assign them. Proposal would be to add a new config key to okgo called release-tag that can be used to specify a release tag older than the newest one that can be set on the build tools. For example, if release-tag was set to go1.7, then even if gödel was compiled using Go 1.8, when it runs the okgo checks it would update the context.ReleaseTags of the global default to only include the tags up to 1.7, which should cause build tag matching to work properly.

The tricky thing here is that, because the checks are run as amalgomated sub-processes, the updating of this global variable needs to occur inside the amalgomated process. Because command-line arguments are the only method of communication from the main process to the amalgomated one, need to figure out a way to do this, either by modifying the checks themselves or adding an interpretation layer to the runner.

Library already generates JUnit XML and go coverage output, so should be trivial to hook up something that generates HTML from that to view in CI environments.

I recently started using the docker task to build images and discovered it would be helpful to pass arguments to the docker build command. In my case, I wanted to use the --build-arg flag to pass in the version of the project being built.

The most flexible solution would probably be something similar to the build-args-script option in the build configuration, but I'd also be happy with a limited option to provide only the project version. Maybe a version-arg: <argument name> option or something?

Make it possible to run tests with race detection enabled.

godelw script currently contains a comparison of the form:

if [ ! $expected_checksum = $computed_checksum ]; then

This will not always evaluate correctly depending on the content of the variables. Should be changed to the safer:

if [ "$expected_checksum" != "$computed_checksum" ]; then

Currently, ./godelw run supports providing arguments and passing them to the program being run. For example, if you have a program that echoes all the arguments, calling ./godelw run foo bar would print [foo bar].

However, this doesn't work for flags -- if you run ./godelw run --prog-flag foo bar, you get the output:

Unknown flag --prog-flag

NAME:
   run - Run a product in the project

USAGE:
   godel run <args>... [flags...]

FLAGS:
   --product   Product to run
   <args>      arguments to pass to product
   --help, -h  print help and exit

This is because the flag is parsed as part of the run command instead of as an argument to the sub-command.

I propose that we fix this by hard-coding a mechanism to distinguish flags. We would do something like:

./godelw run flag:--prog-flag foo bar

When parsing the arguments, any argument that starts with flag: will have that portion stripped. Then, the output would be:

[--prog-flag foo bar]

If the user actually needs to pass a "real" argument that starts with "flag:", they can just stutter it to make it work:

./godelw run flag:flag: foo bar

would produce

[flag: foo bar]

The discoverability of this mechanism isn't great, but at least it would provide a work-around.

In some instances, a user may want to run all of the tests (all tagged and untagged) for a project as one command.

The proposal is to make all a reserved keyword for tag names and make it such that, when all is specified as a tag, all tests that are defined for the project are run.

Repro:

• Create a product that reads from stdin
• Run echo foo | ./godelw run

Expected

"foo" is passed to the stdin of the program being run

Actual

Nothing is passed to stdin

The $ProjectFileDir$ macro that is being generated by gödel no longer works due to https://youtrack.jetbrains.com/issue/WEB-24376. Need to replace with $PROJECT_DIR$/ instead.

Currently, if the curl call in the godelw encounters a page that returns a 400 error code, it will download the content of the returned error page instead of displaying a failure. The call should be updated to fail and display a failure rather than downloading the error page (in which case the result appears as an invalid tgz file that has a checksum that does not match the expected value).

I'd like to have godel print the total time a task took to run when it's finished, just like Maven / Gradle does.

To make things easier locally, I've created symlinks from my regular code locations to the actual source code in GOPATH. This appears to break the error parsing done by okgo. For example, assume we have a project in GOPATH/src/symlink using godel 0.11.4 with the following main.go file:

package main

import (
	"net/http"
)

func main() {
	println("Hello World!")
}

Running ./godelw check compiles fails as expected:

$ ./godelw check compiles
Running compiles...
main.go:4:2: "net/http" imported but not used
main.go:4:2: "net/http" imported but not used

Now, I create a symlink to this directory:

$ cd ~/code
$ ln -s $GOPATH/src/symlink symlink
$ cd symlink
$ ./godelw check compiles
Running compiles...
failed on line 1: line /home/bkeyes/code/symlink/main.go:4:2: "net/http" imported but not used is not valid as the start token for an issue

Because the error is correctly detected and the command also works if I fix the error, I think this is only a problem with the okgo parser (similar to #10) rather than a fundamental problem with using symlinks in this way.

The current implementation requires the user to provide the GitHub upload URL using the --github-upload-url flag if it can't be inferred using the current very simple find-and-replace.

However, since we are creating a release as part of the upload, the release response already contains the upload URL format string, so we should just use that directly instead of requesting it from the user.

Flagged by @jdhenke

The documentation for configuration files is currently typically split between the example test file, the configuration package (which has the annotations that specify the YML keys), and the parameter package (which has the comments/documentation on the entries). Sometimes the package documents are split over multiple files.

Although this may be fine when navigating code, it's non-intuitive and hard to find relevant information when browsing the repository. Should figure out how to centralize and present this information in the docs directory.

I don't think there are docs for getting started (getting a usable godelw added to a project)

Also docs on updating after the getting started would be nice too

In addition to publishing to Artifactory for releases, it would be nice if the publish command supported publishing to GitHub releases as well.

Should be fairly straight-forward to implement -- would basically mirror the artifactory publish action, but would just use the GitHub release API to do so (https://developer.github.com/v3/repos/releases/#create-a-release). It looks like the Go GitHub client has support for all of the required calls (https://github.com/google/go-github/blob/master/github/repos_releases.go).

In 0.9.1 the below dist.yml configuration worked, but in 0.11.1 I don't get any extension key in my manifest:

    dist:
      input-dir: dist/sls
      output-dir: build/distributions
      dist-type-cfg:
        type: sls
        info:
          service-args: "--config var/conf/product.yaml"
          manifest-extensions:
            product-roles: [ 'admin' ]

I'd like a story for publishing a single binary (for a single architecture) without a directory structure. bin is really $name-$version/bin/os-arch[*]/binary where I want an archive just containing binary letting the client choose the os/arch from separate artifacts.

I think we pick from one of a couple options for how to publish to maven:

• Publish multiple raw files with the osarch in the product name: com/palantir/foo/foo-linux-amd64/1.0.0/foo-linux-amd64-1.0.0
  • If we do this we should make sure to publish and check checksum files.
• Publish multiple zip/tgz's containing a single file foo with the osarch in the product name:.
  • com/palantir/foo/foo-darwin-amd64/1.0.0/foo-darwin-amd64-1.0.0.tgz containing foo
  • com/palantir/foo/foo-linux-amd64/1.0.0/foo-linux-amd64-1.0.0.tgz containing foo
• Either of the above but with the os/arch in the version instead of the name:
  • binary:
    • com/palantir/foo/foo/1.0.0-darwin-amd64/foo-1.0.0-darwin-amd64
    • com/palantir/foo/foo/1.0.0-linux-amd64/foo-1.0.0-linux-amd64
  • archive:
    • com/palantir/foo/foo/1.0.0-darwin-amd64/foo-1.0.0-darwin-amd64.tgz containing foo
    • com/palantir/foo/foo/1.0.0-linux-amd64/foo-1.0.0-linux-amd64.tgz containing foo
• Maven has an idea of an artifact "classifier" that's meant for this (multiple artifacts per version) if we want to use it.
  • binary:
    • com/palantir/foo/foo/1.0.0/foo-1.0.0-darwin-amd64
    • com/palantir/foo/foo/1.0.0/foo-1.0.0-linux-amd64
  • archive:
    • com/palantir/foo/foo/1.0.0/foo-1.0.0-darwin-amd64.tgz containing foo
    • com/palantir/foo/foo/1.0.0/foo-1.0.0-linux-amd64.tgz containing foo

I don't really have a strong preference other than standardizing, so decided to lay out a multiple-choice question 😄

Thanks in advance! I think I'd be willing to take a stab at this if nobody wants to beat me to it.

It would be great to have godel support for go generate. Unfortunately for purposes of vendoring, generated code should be checked-in instead of generated on-the-fly.

I think it should work similarly to format where there are apply=true and apply=false modes and it is included in verify. Whitelist directories in a godel/config/generate.yml. generate tasks must result in deterministic, stable output.

It looks like if I have the following git history:

A  <- develop
|
B <- 1.0.0
|
C
|
...

And I run a ./godelw publish on A, then instead of publishing revision 0 to the almanac version 1.0.0-dev, I get a publication that is to revision 1 of 1.0.0

Currently, distgo uses the -C flag when invoking git to set the current working directory. However, not all versions of Git support this -- notably, version 1.7.1 does not seem to have this flag.

There should be a fallback option that works if the -C flag is not available.

If a check like novendor or errcheck fails, the name of the check is printed. However, it's not always clear what the specific check is checking for/how to fix it.

At a minimum, would be helpful if the --help command for a check provided a quick summary of what the check is verifying, and possibly a link to the repository for the check itself if it is a standalone tool such as errcheck.

Logic for evenly distributing tests across workers. Do we have knowledge ahead of time of the number of packages? Number of tests in each package?

something like --test-num-nodes N --test-node-index n

Running ./godelw check takes about 30 seconds for the project I currently work on (5k loc excluding vendored libraries), which is enough of a delay to break my flow. Would it be possible to run at least some of these checks in parallel?

I understand that the mutating checks can't really be run in parallel, but the ones that don't mutate code should probably be parallelizable?

Issue reported on a MacOS X/Darwin system.

1. Create a project that uses a Go library that has platform-specific CGo code (example: https://github.com/mattn/go-sqlite3)
2. Configure distgo to build the project for darwin-amd64 and linux-amd64
3. Run the build command

Expected

Executable should be created for both platforms

Actual

Build for linux-amd64 fails:

# .../persistence
persistence/init.go:28: undefined: sqlite3.SQLiteDriver

Notes

Running GOOS=linux GOARCH=amd64 go build fails with the same error, so this is a general issue with cross-compiling CGo libraries in certain scenarios. Need to figure out the underlying issue and assess what a fix would entail (or whether it's even possible).

If it's not possible to fix outright, need to have a story for how to support building and distributing Go projects that use CGo that want cross-platform compilation.

The go.version property reported in the JUnit test output for tests runs is misleading because it prints the version of Go used to compile gödel/gunit rather than the version of Go that was used to run the tests.

For example, this is the output of tests run against Go 1.8: https://472-75355885-gh.circle-artifacts.com/0/test-results/godel-unit-tests/godel-unit-tests.xml

Example entries:

<testsuites>
<testsuite tests="0" failures="0" time="0.011" name="github.com/palantir/godel">
<properties>
<property name="go.version" value="go1.7.5"/>
</properties>
</testsuite>
<testsuite tests="0" failures="0" time="0.002" name="github.com/palantir/godel/cmd">
<properties>
<property name="go.version" value="go1.7.5"/>
</properties>

This is misleading because the actual version of Go used to run the tests was go1.8.

1. Create a non-main Go package with some files in it (but no tests)
2. Add a file with the following content to the package:

// +build ignore

package main

3. Run gunit commands to run tests in the directory

The command will fail in a non-deterministic manner because the tmp_placeholder_test.go file will sometimes have package main instead of the actual package.

This occurs because, if gunit finds a directory that has multiple packages in it, it will pick one arbitrarily to use as the package for the generated test file.

Two issues here that should be fixed:

1. When there are multiple packages in the same directory, the manner in which the package to use is chosen should be deterministic (sort by package name and use first one instead of depending on map iteration)
2. When a package directory is determined, the build context should be used to attempt to determine the package of the directory so that the build constraints are properly processed. If this fails, then we can fall back to the current approach of picking an arbitrary package.

Convenience command for printing the git version of the project that gets set to version-var. Shouldn't have any other output, the common usage will be

version=$(./godelw project-version)