Skip to content
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

[Maintenance] The Gofrs would like to adopt logrus #799

Closed
theckman opened this issue Aug 6, 2018 · 43 comments
Closed

[Maintenance] The Gofrs would like to adopt logrus #799

theckman opened this issue Aug 6, 2018 · 43 comments
Labels

Comments

@theckman
Copy link

theckman commented Aug 6, 2018

Hi @sirupsen,

A few of us over in the Go community have started a bit of a working group to share the work of maintaining projects that are extremely important to the Go ecosystem. It goes without saying, logrus is absolutely one of those.

We've opened up a way for the community to suggest projects to adopt and logrus was one of them (gofrs/help-requests#2).

I wanted to reach out to see what your preference is for a team like The Gofrs taking over this project. Because this repo is on your personal GitHub account, and not an organization, it seems like we may be fairly limited in the type of shared responsibility we can take on. There are currently 20 members in the organization, so as a first step we could maybe add myself and a few others as collaborators so that we can share the load.

Beyond that, in the interest of full transparency, we may need to consider migrating the project to an organization to allowed richer management of collaborators. That said we can cross that bridge when we get to it.

Cheers!
-Tim

@sirupsen
Copy link
Owner

sirupsen commented Aug 7, 2018

Hi Tim, love the initiative. Unfortunately, I think that migrating to an organization is not worth the cost. I am not saying this out of my ego, I am not thrilled about having my name in all projects—however, judging by the Logrus lowercasegate—I do not want to cause another stir. That already decreased Logrus trust globally significantly. Perhaps you're seeing a path there I am not?

I would be happy to add you as collaborators, however, my biggest priority for Logrus is that it never breaks for anyone. That means maintaining full backwards compatibility of all public APIs, which, unfortunately, due to Go's packaging continues to include my name in the import... 😭

@franciscocpg
Copy link

hi @sirupsen

I understand your worries and it totally makes sense. I was one of many affected by the lowercasegate.

My suggestion is to have an hard fork at https://github.com/gofrs/logrus that starts with version v2. Then we update the README of https://github.com/sirupsen/logrus explaning that this repository will not be maintained anymore pointing to https://github.com/gofrs/logrus

So v1 would use https://github.com/sirupsen/logrus and v2 onwards https://github.com/gofrs/logrus.

In doing so I think we could achieve your priority:

(...) however, my biggest priority for Logrus is that it never breaks for anyone. That means maintaining full backwards compatibility of all public APIs (...)

@sirupsen
Copy link
Owner

sirupsen commented Aug 7, 2018

If we do this, we could certainly discuss some breaking changes for few users (e.g. getting rid of formatters, and just make them hooks) as part of the transition.

If a repository is importing both sirupsen/logrus (through a dependency) and another dependency gofrs/logrus, is there absolutely no way they can conflict even if the package is called the same thing?

@franciscocpg
Copy link

If we do this, we could certainly discuss some breaking changes for few users (e.g. getting rid of formatters, and just make them hooks) as part of the transition.

👍

If a repository is importing both sirupsen/logrus (through a dependency) and another dependency gofrs/logrus, is there absolutely no way they can conflict even if the package is called the same thing?

AFAIK this is a problem only if one file (not a repository) is importing both. Even doing so it's possible to do something like:

import logrusv1 "https://github.com/sirupsen/logrus"
import logrusv2 "https://github.com/gofrs/logrus"

Also, from https://golang.org/doc/code.html#PackageNames:

There is no requirement that package names be unique across all packages linked into a single binary, only that the import paths (their full file names) be unique.

@theckman
Copy link
Author

theckman commented Aug 7, 2018

Hey @sirupsen,

Thank you so much for getting back to me. We really appreciate it.

Hi Tim, love the initiative. Unfortunately, I think that migrating to an organization is not worth the cost. I am not saying this out of my ego, I am not thrilled about having my name in all projects—however, judging by the Logrus lowercasegate—I do not want to cause another stir. That already decreased Logrus trust globally significantly. Perhaps you're seeing a path there I am not?

I think we can both agree that keeping logrus functional and avoiding friction is our top goal. If we cause too much of a headache, what's the use in even maintaining something if people won't adopt it?

To your comment about an organization migration, that actually isn't too disruptive as GitHub will redirect users to the new location (as long as you don't happen to create a new logrus repo). People could keep using logrus at the old location, with no idea that things moved.

However, I see the move as being a long-term possibility if we mutually come to the decision that it's too painful to have a group contribute to the repository as it is today. I'll admit I'm not familiar with what types of permissions you can grant to people contributing to a repository on your account, so it's possible this won't be an issue. (Edit: I am now, and you can't grant them anything in terms of repository administration. 😢)

I would be happy to add you as collaborators, however, my biggest priority for Logrus is that it never breaks for anyone. That means maintaining full backwards compatibility of all public APIs, which, unfortunately, due to Go's packaging continues to include my name in the import... 😭

We're definitely on the same page there.

I'd like to learn more about the API compatibility you want to stick to. Are you against making breaking changes even if it's done with a major version bump (e.g., going from 1.x => 2.x), and then cutting an appropriate git tag? Are you saying that the master branch must remain API compatible?

I think one bias I have is that at this point most consumers of Go are using something beyond pure go get to manage their dependencies. I, and most people I know, have been using dependency managers that can understand semantic versioned SCM tags when pulling dependencies in for years. This means I'm more willing to make very few, yet thoughtful, breaking changes gated behind a major version bump.

To the point made by @franciscocpg, I'd personally much prefer to avoid a hard fork if possible. I find it extremely valuable to retain the issue and PR history of a project. There are sometimes very useful bits of context in there that would otherwise be lost. It's why I prefer to either contribute where it currently lives, or to migrate to a new location and let us cheat using GitHub redirects.

@franciscocpg
Copy link

franciscocpg commented Aug 7, 2018

@theckman
To be honest I don't have a deep knowledge about how github migrations works (I mean: I've never done this before).
As you said that github takes care of redirects I think that's a good option.

@dgsb
Copy link
Collaborator

dgsb commented Aug 8, 2018

Hello, I could not find any documentation for migrating a repository from a user to an organization. Could someone post here a documentation referencing the process of such a migration ?
Currently, logrus is directly imported by several thousands other packages, I don't think it is realistic to plan a migration if the import path has to change.

@theckman
Copy link
Author

theckman commented Aug 9, 2018

I believe these are the docs you're looking for:

This article also mentions the one limitation I called out a few messages above:

If you create a new repository under your account with the same name as the transferred repository, existing redirects to the transferred repository will break. Instead, use a different name for the new repository.

This means that so as long as a new logrus repo isn't added to the sirupsen user on GitHub, they will redirect requests to the new location. I have seen go get, and others, work fine with these redirects. If you really would like to see this in action, I can set up an example project and move one of my personal repos to The Gofrs organization. That way the example project can show you that it's able to import it just fine.

Currently, logrus is directly imported by several thousands other packages, I don't think it is realistic to plan a migration if the import path has to change.

I don't think anyone in this thread is advocating for a move right now, and I tried to be explicit about that in my original comment:

Beyond that, in the interest of full transparency, we may need to consider migrating the project to an organization to allowed richer management of collaborators. That said we can cross that bridge when we get to it.

I wasn't looking to discuss it now or make it part of the plan, as mentioned by saying "we can cross that bridge when we get to it". However, since it seems like it's become a focus of this discussion it's probably best that I transparently share my thoughts on why a migration may be something we need down the road.

Before doing that though, some context is that The Gofrs is being built as a team of people to take on the maintenance of repositories. The big reason for this being a team effort is that I don't want the burden of project maintenance to fall on a single person, even within The Gofrs. We want it to be a shared responsibility to try and avoid burnout and to provide different sets of perspectives on things.

Having said that, with repositories on single user accounts only that user (e.g., sirupsen) can manage the collaborators. So that means anytime someone in The Gofrs wants to start maintaining that repository, we'd need to reach out to the owner to have them add them as a collaborator. The same is true if someone is no longer maintaining the project, we want to remove their write access to avoid any accidents. The owner of the repository, the person who says they do not have the resources to maintain the project, would then always need to be on standby to make these changes. What happens if they are in the Maldives for two weeks without their laptop?

When you consider that last scenario, it's a bit scary to think that we on The Gofrs would not be able to revoke commit access if a user's account was somehow compromised. We require the use of two-factor authentication to be in our organization which reduces that risk, but we cannot make the same enforcement on a repo on a single user's account. So a user with contributor access could disable two-factor authentication and still have commit access to that repo. We've configured The Gofrs organization to disallow that.

@dgsb The thing I'm now curious about, if you consider the thousands of people who import logrus, do you feel these are acceptable security risks? If you assume that old imports will still work, and updating your imports is an opportunistic thing, is it worth considering the benefits of a move? I'm not trying to pick on you, and am instead wondering how these types of risks contribute to your perspective.

@matt0x6F
Copy link

matt0x6F commented Aug 9, 2018

Just an idea for backwards compatibility, couldn't a mirror be setup for the old repo to the new repo? This would make sure inattentive users can still get logrus without thinking it's been abandoned.

@franciscocpg
Copy link

franciscocpg commented Aug 9, 2018

In #799 (comment)

This means that so as long as a new logrus repo isn't added to the sirupsen user on GitHub, they will redirect requests to the new location.

This seems so fragile. What happens if sirupsen create a new logrus repo by mistake or just because he wants?

@theckman
Copy link
Author

theckman commented Aug 9, 2018

@franciscocpg if he's aware we have moved the repo and he shouldn't create a repo with the same name, what do you believe the likelihood that it'll happen? Do you think it would happen without that person engaging the people who moved the repo first?

To provide some context, looking at this issue, and the project in general, it's clear to me that Simon cares about people's ability to use the package and the overall success of the project. Considering that I don't anticipate what you mention being a large risk.

@franciscocpg
Copy link

I see. Just to be clear. I'm not trying to judge sirupsen's character or something like that.
My point is just that IMO technically it "seems" fragile.

But maybe it worth the trade-off.

@theckman
Copy link
Author

theckman commented Aug 9, 2018

@franciscocpg I think it was a great question to ask, and I didn't personally read it as a judgement of character. I was only hoping to share my context as to why I didn't perceive it as being too risky as a long-term option.

@franciscocpg
Copy link

franciscocpg commented Aug 10, 2018

Still about

This means that so as long as a new logrus repo isn't added to the sirupsen user on GitHub, they will redirect requests to the new location.

Another thing that comes to my mind now:
This also means that @sirupsen would be prevented from forking gofrs/logrus repo.
Wouldn't it be a problem?

@theckman
Copy link
Author

@franciscocpg Maybe. Some of that may be mitigated by making him a contributor on our repo, since he could push branches and open PRs.

@sbuzonas
Copy link

This also means that @sirupsen would be prevented from forking gofrs/logrus repo.
Wouldn't it be a problem?

A fork can be created in another account, renamed, and transferred. I've done this in the past to prevent clobbering redirects.

@dgsb
Copy link
Collaborator

dgsb commented Aug 21, 2018

For the record here is the documentation page which describes the redirect upon repository transfer: https://help.github.com/articles/about-repository-transfers/

@sirupsen
Copy link
Owner

sirupsen commented Sep 2, 2018

Sorry for the silence here. Had a bunch of things come up.

I am extremely committed to get this into your capable hands, I just want to ensure we don't break anything. I wouldn't create a new repo if we can transfer it with a redirect, it seems good to me. I don't want to create another logrusgate-type thing. I am happy to completely step back once we decide on the best transition.

I agree that today, most are using modern version control for Go. What I worry about are the 100s to 1000s of gists lying around that would no longer work if we broke compatibility with master. Logrus is treated a bit like stdlib in that regard. People assume that they can import master, and go.

What if we maintained 1.x full compatibility in master (even post-redirect) and then did 2.x in tags outside of master? Is that crazy?

@dgsb
Copy link
Collaborator

dgsb commented Sep 2, 2018

As we receive some backward incompatible pr from time to time, I've created this issue #815 in order to reach consensus.

@sirupsen
Copy link
Owner

sirupsen commented Sep 3, 2018

To me, that's the key decision here. If we can agree on that structure, I see no issue making this handle permanently a redirect and update the README with the new governance and repository structure.

@theckman
Copy link
Author

theckman commented Sep 5, 2018

@sirupsen no worries; GopherCon prep had me busy as well. There is one case[1] I know of where a simple repository migration won't work, but I've confirmed logrus doesn't fall in to that bucket.

In having some discussions with people in Denver around this effort, I think there's value in doing some small experiments with migrating a repository of mine to the organization to re-validate my earlier observations. I plan on moving one of my projects this week, and making sure things work as-expected.

What if we maintained 1.x full compatibility in master (even post-redirect) and then did 2.x in tags outside of master? Is that crazy

I don't think it's crazy, because it would keep that guarantee. However, I don't know how well that will play with the modules system introduced in Go 1.11. I can also see it causing confusion with some people opening PRs/issues against the wrong branch, or code diving and spending time grokking a different implementation.

[1] The one case I know of is when your project has an internal directory. The migration causes the compiler to think package A is trying to import B/internal, even though A was renamed to B and GitHub is redirecting it. Again, logrus doesn't use an internal/ directory so it should be fine.

@sbuzonas
Copy link

sbuzonas commented Sep 6, 2018

I can also see it causing confusion with some people opening PRs/issues against the wrong branch, or code diving and spending time grokking a different implementation.

Much of that can be mitigated by making a version branch like 2.x and updating the repository HEAD to point to the latest version branch.

@theckman
Copy link
Author

theckman commented Sep 15, 2018

It took a little longer than I anticipated to get the one outstanding PR merged, but I've done the migration now.

The original project was hosted at https://github.com/theckman/go-flock. I then migrated it to the gofrs organization, and it became https://github.com/gofrs/go-flock. I then renamed it from go-flock to flock, so now it's at https://github.com/gofrs/flock.

GitHub has set up redirects to send the original two URLs to https://github.com/gofrs/flock.

@trusch
Copy link

trusch commented Jan 4, 2019

I would really like to see that happen, since its currently impossible for me to import github.com/sirupsen/logrus because of the sS-problem + the extreme slowlyness of the moby folks to update the import pathes in the docker repos.

With this migration (and especially the v2) I could finally make all my projects work with go modules + logrus.

@theckman may I assist to get this going again?

@thaJeztah
Copy link
Collaborator

@trusch where are you seeing the old (S) path in the moby repository?

@theckman
Copy link
Author

theckman commented Jan 5, 2019

@trusch I'm currently on vacation in Japan, and part of my time is going to be spent working on personal projects (while relaxing home with my wife's family). I'm also going to be budgeting out specific hours each week to dedicate my time to our projects moving forward. So I'm going to start working towards this in the coming days, with dedicate time to continue working on it, and so depending on what help I need I'll be in touch.

@theckman
Copy link
Author

theckman commented Jan 6, 2019

@thepudds @myitcv is there a story / user experience around what happens when a project is moved on GitHub WRT modules? Are things going to start breaking for them if they use modules, because their import statement won't match the import listed in the go.mod file?

@trusch
Copy link

trusch commented Jan 6, 2019

@thaJeztah I see it in the 17.05.0-ce tag and the latest v1.* Tag which is considered the latest stable tag following semver (this gets automatically selected by go mod tidy)

@thaJeztah
Copy link
Collaborator

I see it in the 17.05.0-ce tag and the latest v1.* Tag which is considered the latest stable tag following semver (this gets automatically selected by go mod tidy)

Right, so there's a couple of issues there;

  • go mod assumes that any tag is a SemVer (the moby / docker repository has never used SemVer, so that won't work).
  • The second issue is that since the introduction of the Moby project, no new releases have been tagged in the moby repository, so the tag that's found in that repository is the last release before the introduction of the moby project. The moby repository now acts as an upstream for Docker, and releases of the Docker are done from their downstream repositories. The https://github.com/docker/docker-ce/ repository is for releasing the packages, and bundles all the components (engine, cli, packaging scripts) that make up a release, and a repository for the "engine" component (which is the downstream fork of the "moby" repository); https://github.com/docker/engine. If you want to vendor a version of the "engine" component that matches a specific Docker release, you can vendor from that repository; https://github.com/docker/engine/releases, but you'll have "import" it as github.com/docker/docker (I think when using Go modules, that's done through a replace in the configuration)

@theckman
Copy link
Author

theckman commented Jan 14, 2019

Okay so I've been doing some digging here, and with modules support enabled now it seems like our original migration plan is no longer possible. It does mean we need to come up with an alternative plan. This totally was my fault and I'm really sorry. 😞

In summary because the go.mod file defines the module's import path, the plan we were going to use before to just migrate and rely on GitHub redirects to help us will no longer work. To avoid breaking consumers, I think we will need to retain a github.com/sirupsen/logrus repository with the current go.mod intact.

I'm doing a bit of traveling to visit family in Japan so I've not been able to spend my usual amount of time thinking about it, but so far there are a few options.

Option A

  • migrate this repository to the Gofrs like we originally intended
  • sirupsen will then need to make a clone of that repo (github.com/gofrs/logrus) back under their user
  • sirupsen (or someone with access) will need to push a commit to update the README indicating it's deprecated / a link to the new location
  • sirupsen (or someone with access) will need to mark the repo as archived in the GitHub UI

The end result is that this repository, with full issue/pr history, would be located at github.com/gofrs/logrus.

That said, there are some cons with this approach:

  • During the migration period, things will fail to resolve github.com/logrus/sirupsen (less than 5 minutes)
  • All current github.com/sirupsen/<issues|pull> links will be broken.

Option B

The only other option we would be to fork the repo, request GitHub support detach it from sirupsen/logrus, and then add a comment to each issue / PR to request they re-open it under the new repository. This will keep all links working, avoid the downtime during the migration, but would put a lot of work on those who reported issues / opened PRs.

One upside of the toil, if you can call it that, is it'd be a good filter for issues/pull requests. If it doesn't get reopened, maybe it wasn't that important.

Option C

I started a conversation here, where @bcmills made a suggestion:

If you want people to be able to import using the new path instead, then you can do that with a forwarding package (and I would recommend bumping the major version too, so that you don't have two modules with different import paths trying to resolve using the same set of tags).

So github.com/sirupsen/logrus (tagged v1.4.0) could be a thin package that forwards to (I'm assuming) github.com/gofrs/logrus/v2 (tagged v2.0.0), and that should work even if github.com/sirupsen/logrus and github.com/gofrs/logrus resolve to the same underlying repository.

(If that doesn't work, please file an issue and ping me.)

I'm not sure I fully understand how we'd accomplish this on the master branch without breaking people who catch us in the middle of the transition. It sounds like they'd get v1.4.0 it'd reference the new /v2 import which wouldn't exist yet, 💥.

I'd love to hear the thoughts of others on this one. I'd also be good if those more experienced with modules can 👀 this as well.

@thepudds
Copy link

thepudds commented Jan 14, 2019

@theckman There are obviously some module-specific pieces here, but to help level-set on the nature of what some of the problems are, I wanted to briefly mention that there is some overlap with how before modules the go tool would enforce consistency between a consumer's import path and the declaration of the publisher's desired import path when a publisher would declare the required import path using import path comments on a package statement.

In other words, taking a simpler pre-modules example, uber couldn't just tell people to use a new vanity url for zap without doing anything else, because zap's current import comments would cause consumers to break (even pre-modules) if a consumer started using an import path other than go.uber.org/zap. Right now, zap declares:
package zap // import "go.uber.org/zap"
https://github.com/uber-go/zap/blob/master/doc.go#L11

I think part of the intent from uber's perspective is to prevent someone from accidentally importing github.com/uber-go/zap in one spot but go.uber.org/zap elsewhere (e.g., in another consumer package in the same build).

The module directive in a go.mod file (usually the first line in any go.mod file) serves a similar function as the declaration of how consumers must import the code.

This is not any advice or suggested solution, but wanted to re-cap some of the behaviors to consider as different solutions are evaluated.

edit: some related snippets from the official go command documentation:

https://golang.org/cmd/go/#hdr-Import_path_checking

The go command will refuse to install a package with an import comment unless it is being referred to by that import path. In this way, import comments let package authors make sure the custom import path is used and not a direct path to the underlying code hosting site.

Import path checking is also disabled when using modules. Import path comments are obsoleted by the go.mod file's module statement.

edit 2: first cut was slightly mangled. Sorry.

@bcmills
Copy link

bcmills commented Jan 14, 2019

I'm not sure I fully understand how we'd accomplish this on the master branch without breaking people who catch us in the middle of the transition.

I think you'd have two possible approaches, depending on how far back you want compatibility and a couple of other factors. Using the major-subdirectory layout, you can do the whole thing in one commit, so that's what I'll describe first — it's probably simplest.

You can tag that commit as both v1.4.0 and v2.0.0, since the contents of both major versions are stored in separate subdirectories.

At the repo root, you'd want something like:

/go.mod:

module github.com/sirupsen/logrus

require github.com/gofrs/logrus/v2 v2.0.0

/forward.go:

/*
Package logrus is a structured logger for Go, completely API compatible with the standard library logger.
[…]
*/
package logrus  // import "github.com/sirupsen/logrus"

import (
	v2 "github.com/gofrs/logrus/v2"
)

// […]
func Info(args ...interface{}) {
	// Might need to add a “depth” variant in v2 to pick up the right caller info;
	// see glog.InfoDepth for an example.
	v2.InfoDepth(1, args...)
}

// […]
type Logger = v2.Logger

// […]
func New() *Logger { return v2.New() }

[…]  // etc.

/v2/go.mod:

module github.com/gofrs/logrus/v2

// Ensure that if some other library imports sirupsen/logrus,
// they get the forwarding package instead of a completely separate implementation.
require github.com/sirupsen/logrus v1.4.0

[…]

/v2/doc.go:

/*
Package logrus is a structured logger for Go, completely API compatible with the standard library logger.
[…]
*/
package logrus  // import "github.com/gofrs/logrus/v2"

/v2/[…]
(Move the other source files as appropriate.)

@bcmills
Copy link

bcmills commented Jan 14, 2019

In theory the goforward tool in https://golang.org/cl/137076 should do most of that change for you, but at the moment it doesn't. I think I need to modify it to allow cross-module moves as long as both directories are writable.

@bcmills
Copy link

bcmills commented Jan 14, 2019

Actually testing that change would be tricky, unfortunately: you'd need to use replace directives in both modules to mimic the circular dependency, since there is currently no way to push unpublished commits into the module cache (see golang/go#28835 for one proposal to address that).

@bcmills
Copy link

bcmills commented Jan 14, 2019

Note that you can implement forwarding regardless of whether you start a new (forked) repo or move the existing one. (It's not so much “option C” as “A with forwarding” or “B with forwarding”.)

In my opinion, forwarding is only viable way to change the import path: options A and B would both break the semantics of logrus.AddHook, since hooks registered at one path would otherwise not be visible to the version of the package at the other.

@bcmills
Copy link

bcmills commented Jan 14, 2019

If you want to avoid moving the files into a /v2 subdirectory, you could do the same thing using the major-branch model, but it would be more complicated for folks in GOPATH mode to always end up with the correct version — and more complicated to test, since you'd have to use two copies of the repo to set up the replace directives.

With that approach, you'd issue one commit tagged v1.4.0 (probably on a v1 branch?) that converts the entire package to forward to github.com/gofrs/logrus/v2, and second commit tagged v2.0.0 (on master, or perhaps on a v2 branch) that changes the module path from github.com/sirupsen/logrus to github.com/gofrs/logrus/v2 and updates the internal imports to use that path. You'd want to publish both commits at the same time, since v1.4.0 would require v2.0.0 and vice-versa.

@theckman
Copy link
Author

@bcmills thank you for taking time to reply and explain our options. That final option may be the best result, but probably worth simulating to make sure we're happy with the result. So:

Option D

Option E

@thepudds
Copy link

thepudds commented Jan 14, 2019

@bcmills wrote:

If you want to avoid moving the files into a /v2 subdirectory, you could do the same thing using the major-branch model
...
With that approach, you'd issue one commit tagged v1.4.0 (probably on a v1 branch?) that converts the entire package to forward to github.com/gofrs/logrus/v2, and second commit tagged v2.0.0 (on master, or perhaps on a v2 branch) that changes the module path from github.com/sirupsen/logrus to github.com/gofrs/logrus/v2 and updates the internal imports to use that path. You'd want to publish both commits at the same time, since v1.4.0 would require v2.0.0 and vice-versa.

This might be possible as an alternative to publishing both commits at same time:

  1. If you create a v2 branch, get it ready with a go.mod with a require logrus v1.4.0-migration, then tag the v2 branch with a tag v2.0.0-migration tag (without yet touching master).
  2. Practice next step, but then..
  3. Update the go.mod in master to have require logrus v2.0.0-migration and tag master as v1.4.0-migration

And at that point, it is live (without needing to have committed to a v2 branch and master at the same time).

At some later date, as new tags get applied without -migration, the require statements in the go.mod files could be updated to use those newer tags where those newer tags would no longer have -migration in them.

Also, a v1 branch could also likely be used in place of master in those steps above, at least to start.

@dgsb
Copy link
Collaborator

dgsb commented Jan 16, 2019

Hey guys, I don't want to sound harsh, but what you want to do seems highly complicated with no clear added value to me.
It would be so much more useful you spend some of your time helping triaging the issues.
From an outside view the activity in the gofrs organization seems to have stalled, so before migrating this project to gofrs, it would be nice to show this group is indeed working on the project.

@theckman
Copy link
Author

theckman commented Jan 16, 2019

@dgsb the value add is that we can have multiple users with administrative access to the repo.

Edit: Unless it changed recently, contributors cannot be administrators if the repo is on a single user's account.

@thepudds
Copy link

thepudds commented Jan 16, 2019

Setting aside for the moment the question who fixes what where...

On the technical aspects of the migration question, my perhaps incorrect understanding is:

  • @theckman and the gofrs organization have experience doing this type of migration using github redirects (e.g., github.com/theckman/go-flock now redirects to github.com/gofrs/flock).
  • They were also hoping to migrate logrus in the same way (if things were to move forward with the overall transfer proposal, of course).
  • They hit what they believe is a snag because the go.mod file in logrus declares itself as module github.com/sirupsen/logrus...

If that is correct, then a simpler option (compared to some of the options discussed in the comments above) might be something like:

  1. do the redirect exactly as they planned, but
  2. have github.com/gofrs/logrus/go.mod continue to read module github.com/sirupsen/logrus. (In other words, don't change the required import path specified in the go.mod file).
  3. people will still need to do import "github.com/sirupsen/logrus" (just as they do today).
  4. the plugins in the logrus ecosystem continue to import "github.com/sirupsen/logrus", so they in theory keep working.
  5. people should not import github.com/gofrs/logrus (similar to how zap users should not import github.com/uber-go/zap, but should instead import go.uber.org/zap)

In other words, if that is acceptable, and if that works, that might be simpler than any type aliasing / forwarding / goforward approach?

I think that would be similar in spirit to how some projects in the past like zap purposefully disallow someone from (accidentally) having two separate versions of their code in someone's build by enforcing a single, canonical import path via "import comments" (sometimes called "import pragmas" or "import path enforcement"):

package zap // import "go.uber.org/zap"
https://github.com/uber-go/zap/blob/master/doc.go#L113

Under the option outlined in this comment, I think then any discussion about a v2+ version could be a separate / future matter?

All that said, I am not intimately familiar with the history of logrus, so sorry if any or all of this is off base...

@theckman
Copy link
Author

@thepudds thank you for taking the time to write that up. I think that final option is our best bet, and would be the one we'd likely pursue. In doing some research we found the error the modules code presents when the module path differs isn't very actionable[1].

I've just raised a CL[2] to fix an issue with the unexpected module path error to be a bit more actionable. If that change, or a similar one, gets merged it should improve the error in a way that would let people know what they have to do to fix it being the wrong import.

I think that would then give us the cleanest migration path forward.

[1] golang/go#28489
[2] https://go-review.googlesource.com/c/go/+/158477

@stale
Copy link

stale bot commented Feb 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 26, 2020
@stale stale bot closed this as completed Mar 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants