nim-lang / nimble

Package manager for the Nim programming language.
https://nim-lang.github.io/nimble/index.html
Other
1.25k stars 191 forks source link

Implement `tag` command #525

Open dom96 opened 6 years ago

dom96 commented 6 years ago

Usage: nimble tag v0.1.1

Nimble will run git tag v0.1.1 (or equivalent) and verify that the package's .nimble file has been updated with that version.

Bonus feature:

zevv commented 5 years ago

I guess having nimble editing the .nimble file is a no-no? It would be convenient if one command would do all the bookkeeping needed for a release - updating nimble,making the tag and pushing the repo all in one go.

PMunch commented 5 years ago

Problem with that though is that the nimble file is NimScript and IIRC you can do things like version = staticExec("git describe --tags") so the logic for what to change might be a bit difficult to get right..

kristianmandrup commented 4 years ago

Let's add this command feature ASAP. All other package libraries I know of do this sort of tagging as part of publish. We could even add the following convenience commands:

$ nimble bump patch # increase version by `0.0.1` and run `nimble tag`
$ nimble bump minor # increase version by `0.1.0` and set patch version `0`, then run `nimble tag`
$ nimble bump major # increase version by `1.0.0` and set patch + minor version `0`, and run `nimble tag`

I'll give it a go

kristianmandrup commented 4 years ago

I added initial support for the tag command here. Currently only supports git

Tag repo and push tags to remote repo

$ nimble --tag 0.1.0

Increase patch version tag and publish (ie. 0.1.2 to 0.1.3)

$ nimble publish --tag patch
# shorthand alternative
$ nimble publish --tag .

Increase minor version tag and publish (ie. 0.1.2 to 0.2.0)

$ nimble publish --tag minor
# shorthand alternative
$ nimble publish --tag m

Increase major version tag and publish (ie. 0.1.2 to 1.0.0)

$ nimble publish --tag major
# shorthand alternative
$ nimble publish --tag M

The increase tag variant works for the tag command as well:

$ nimble tag --tag M

Perhaps the option should be renamed version though

kristianmandrup commented 4 years ago

Now uses bump under the covers. Could use some optimisation, leveraging bump way more as it is super powerful

genotrance commented 4 years ago

Main feedback is that you cannot depend on bump as a package since nimble is the package manager and you cannot install bump before nimble is built.

https://github.com/nim-lang/Nim/blob/devel/koch.nim#L163

dom96 commented 4 years ago

At this point, why don't we just make nimble bump an alias to bump? Better yet, Nimble should have the ability to delegate functionality to executables named nimble-<command_name> so bump would just need to offer a nimble-bump binary.

kristianmandrup commented 4 years ago

We could just execute bump in the shell if nimble is provided bump arguments or perhaps a bump subcommand?

nimble publish --bump patch

Would make it much easier to implement and stay in sync with bump functionality

kristianmandrup commented 4 years ago

I've implemented a new version of this feature where I've removed the dependency on bump. Made a number of additional improvements. Pls check tag-command branch or latest commits on the branch

Needs testing, unit testing (tmrw?) and review. Likely the options format need to be agreed on for optimal UX

Should I open a PR to work from?

dom96 commented 4 years ago

We could just execute bump in the shell if nimble is provided bump arguments or perhaps a bump subcommand?

Yes, again, I'd like a generic solution where packages can provide binaries named nimble-<name> and then Nimble can execute them via nimble <name>. That way the bump package could provide a nimble-bump package and interoperate effortlessly, the user would just need to install it (we can give hints for this for some popular functionality)

Araq commented 4 years ago

named nimble- so bump would just need to offer a nimble-bump binary.

Please don't. Either make "bump" a real feature of Nimble or stay out of bump's way. "Every command must start with nimble-" doesn't add any usability whatsoever.

kristianmandrup commented 4 years ago

@Araq I also recommend keeping nimble dependency free. My latest commit has no dependencies. bump can be used on the side. A reference + note about bump could be added to the nimble Readme perhaps. Just my thoughts. Cheers.

dom96 commented 4 years ago

"Every command must start with nimble-" doesn't add any usability whatsoever.

Huh? It obviously adds usability, it would make these kinds of features trivial.

genotrance commented 4 years ago

@dom96 - main push back from me is that nimble bump is longer than bump. Having a generic feature in nimble would be interesting if there were many such external sub-commands. git does something like this right?

Options:

cc @disruptek

disruptek commented 4 years ago

I don't really have much of an opinion, except that it seems completely irrelevant to the compiler and probably even orthogonal to the distribution. Pulling it into nimble would allow me to deprecate it as a separate tool, so that would be nice.

I think dynamic subcommands are a cute feature, but only if the parent and child apps actually benefit from the integration. I don't really see that here, but maybe you guys have a good idea that hasn't occurred to me.

For example, bump functions will get absorbed into nimph so that

...and so on...

disruptek commented 4 years ago

As I said in IRC, I'm flattered that Kristian took the time on this PR. I also think the fact that his time is now costing us time and creating pushback is absurd.

Let's close this PR with apologies. I agree with @dom96 that automatic subcommand extensions makes sense. It lets subcommands develop independently, removes friction from the ecosystem, and stifles duplication of effort like this.

Let's also make a way to query nimble for its available subcommands and include their (abbreviated) --help output in nimble's. Maybe we can also create a tag for nimble subcommands so that they are more easily found via a nimble search. Then we can advertise that in the software and online, somewhere, too.

kristianmandrup commented 4 years ago

Thanks for the thumbs up. Please know that the PR has not been fully tested and is mainly a POC. I'm still very new to the Nim ecosystem, so perhaps better that someone take charge, test + polish it, to finalise this PR. How about the CLI arguments? should it be --bump patch or perhaps use tag or version?

dom96 commented 4 years ago

@dom96 - main push back from me is that nimble bump is longer than bump

This push back doesn't make sense to me. If nimble bump works (by calling nimble-bump from the bump package) then executing bump will work too since the Nimble package will continue to expose it as its own binary (i.e. you'll have two binaries, nimble-bump and bump), so the user can use it directly too if they wish.

kristianmandrup commented 4 years ago

I think it would be a great idea to integrate bump with nimble. The main question is how to do this and how to provide the best user experience.

Sub-commands would likely be the way forward, as it opens up more flexibility. Perhaps some of the other commands should use sub-commands as well?

In terms of requiring pre-existing installation of bump that would make it much easier to do obviously and bump could then evolve independently. Let's try it. Much easier pathway than building new functionality or integrating bump directly into the core of nimble. Your thoughts?

genotrance commented 4 years ago

nimble bump simply calling nimble-bump would simplify the design within Nimble but won't make the user experience any better. If designed generically, nimble won't know that nimble pump is dumb or that the user needs to do something for nimble bump to work (i.e. nimble install bump).

If nimble instead maintains a list of known subcommands, when a user runs nimble bump, nimble would know that it needs to download the bump package if not already installed. Then the user experience would be seamless and this effort would be worthwhile.

So no need to changes bump to add a nimble-bump target since nimble would know that nimble bump should findExe("bump") and nimble install bump. And once this capability is enabled, adding new sub-commands would basically need a line of code pointing to the package and corresponding binary name.

Next question is when nimble should update an existing install of bump to the latest.

dom96 commented 4 years ago

Much easier pathway than building new functionality or integrating bump directly into the core of nimble. Your thoughts?

If that question is aimed at me then you should already know my answer: yes, I would prefer to keep Nimble simpler and allow its evolution via external commands.

If nimble instead maintains a list of known subcommands, when a user runs nimble bump, nimble would know that it needs to download the bump package if not already installed. Then the user experience would be seamless and this effort would be worthwhile.

This approach doesn't scale. One thing we can do is mark packages providing a binary that Nimble can call with a nimble-command-<cmd_name>. Then when Nimble is executed with a command it does not understand it can look for packages with those tags and list them as candidates for installation.

But this is something that can come after, it's an additional feature that makes discovery better, but it's not necessary. Let's focus on the basics first. For a basic implementation I think a nimble- prefix to the binary is a must, it will also allow commands to be run faster than having to look up the installed packages (when we do implement the tagging logic I described above).

Next question is when nimble should update an existing install of bump to the latest.

I don't think we should worry about this for now. This is another feature that is much further in the future and we can cross that bridge when we come to it.

genotrance commented 4 years ago

Related to https://github.com/nim-lang/nimble/issues/433 which is the generic requirement.