Submodules for pony-semver, ponylang/appdirs; using appdirs for repo cache

[cquinn]

Migrated use of pony-semver to a submodule referencing btab/pony-semver @ v0.1

• Fixes #77

Added submodule for ponylang/appdirs.

• Fixes #78

Using appdirs now to compute default repo cache directory.

• Fixes #28

Added a --repo_cache_dir option to allow custom repo cache locations.

[SeanTAllen]

• This needs manual CHANGELOG entries.

And one of:

• The Makefile needs to be updated to check out git submodules if they don't already exist.
• The checkout needs to be done in a recursive fashion to get the submodules

I'm leaning towards the Makefile should use dependency tracking to determine if the submodules are checked out and if not, should check them out. However, to move along the PR and verify that tests pass etc, Then the various github workflows that use the checkout action should be updated like this:

uses: actions/checkout@v1
  with:
    submodules: recursive

We should discuss which approach we think is better- Makefile or at checkout during sync. I tend to favor the Makefile approach as it means that folks don't have to know to do a recursive fetch of submodules when building locally as that dependency management will be handled for them. This is the same that I think that stable fetch or corral fetch should be handled in Makefiles for projects.

[SeanTAllen]

I'm not particularly comfortable with the change to using an external dependency that we don't control.

And important issue. It's referencing a tagged commit. I think this is a bad idea. Security-wise, that tag can be moved. It should use a SHA because, its a repo that is out of our control. A tag can be moved and introduce an attack vector. A SHA will still point to the same commit. That said, I'm not really comfortable using a library that is external. I'd prefer to develop a ponylang alternative or if @btab is willing, to make pony-semver a Ponylang project.

It's fine that appdirs is using a tag as it is a ponylang project that we control.

[cquinn]

I have updated the Makefile to pull the submodules as a dep. This works locally, but I am not sure if it addresses all the paths that the github workflows need. I've updated the pony-semver reference to use the SHA. I will check with @btab to see if he is interested in moving pony-semver into ponylang.

[cquinn]

I updated the github workflows that were building Docker images to ensure that the submodules are pulled at checkout time. And I updated the Dockerfile to get the submodule trees copied into the build container. But I now see an error from make: make: *** No rule to make target 'install'. Stop. locally and on github that is mysterious.

[SeanTAllen]

So, you don't need both the submodule recursive in github workflows and also in the Makefile. You should pick one. 2 is redundant.

You didn't update all the .github workflow files that need updating so if you want to go that route, that needs to be completed.

The submodules support is incorrect how it is done. I could start to fix it, but it also won't work particularly well for anything other than pulling in the dependencies. Really it should also be scanning for .pony files in the directories and using as part of a build target. Other than the existing syntax problem, the current setup would allow for a change to a dependency in the subdirectory and wouldn't trigger a rebuild. But if you changed the README for either, it would trigger a rebuild.

[cquinn]

I see that both the git workflows and Makefile approach are redundant in the CI use cases. But they are not necessarily in developer use cases. I guess we could add a note to the readme to remind people to git clone --recursive and/or git submodule update --init. I took the approach of relying on the Makefile for make-based targets but added the --recursive to the workflows that ran Docker build directly which then requires the submodules to be pulled prior to the Docker build.

The Makefile implementation for pulling the submodules is strictly for pulling them the first time to make it more developer-friendly. Maybe that limited solution is more confusing than helpful and we should just rely on instructions in the readme.

[SeanTAllen]

I think we should be doing the Makefile approach, but neither was complete when I left that comment. The recursive ci pull wasn't done for all cases that needed it and Makefile left a lot of edge-cases around and wasn't working.

I think you should finish off the recursive CI stuff (it needs to be done everywhere in a .github workflow that uses checkout@v1) and remove the Makefile stuff for now and we can add an issue to add full and correct handling in the Makefile.

[SeanTAllen]

I'm going to take over finishing off this PR unless someone else wants to.

I definitely want to see Carl's last work completed and merged.

[SeanTAllen]

I did implement most of these changes.