still Doesn't support shard renaming and CAN be fixed.

[masukomi]

despite issue #272 being closed this is STILL a bug in 0.9.0 and it absolutely can be fixed (potential solution below)

This is a serious issue because it breaks 3 things:

• the ability for a shard owner to rename it (not horrible)
• the ability for a shard owner to recover from accidentally releasing a version with ANY change to the name (accidentally committed typo)
• the ability for anyone to fork a shard and change the name unless they ALSO wipe out all the previous tags.

If the name changes (intentionally or accidentally) this shard can no longer be installed, nor can any future forks that attempt to rename it to make it clear it's a fork.

The problem is that this method

def add(dependency : Dependency)

in src/solver/graph.cr

throws an error unless dependency.name == spec.name

This is a bug because it does it for every historical version and it is comparing them against what was specified. Thus if the past is different from the present an exception is raised.

Here's a potential fix. The assumption of this fix is that we don't care about versions where the name doesn't match. Maybe it was the old name. Maybe there was an accidental typo committed. Let's just acknowledge it and move on. Eventually (in theory) we'll get to a version where the name does match.

      def add(dependency : Dependency)
        pkg = @packages[dependency.name] ||= Pkg.new(dependency)
        resolver = pkg.resolver

        versions_for(dependency, resolver).each do |version|
          next if pkg.versions.has_key?(version)

          if spec = resolver.spec?(version)
            if dependency.name != spec.name
              STDERR.puts("WARNING: shard name (#{spec.name}) doesn't match dependency name (#{dependency.name}) in version #{version}.\n   Skipping this version.")

              pkg.versions[version] = spec
              add(spec)
            end
          else
            # skip (e.g. missing shard.yml)
          end
        end
      end

I haven't made a patch for it because it causes all sorts of failures in the tests.

I think the correct solution should probably also throw an error if NO versions have been added.

[ysbaddaden]

This is a bug because it does it for every historical version

This isn't a bug. Renaming a shard isn't supported, for reasons explained in #272.

Skipping versions for non matching names is interesting... but I can foresee a bunch of issuesnwith this approach. For example, what if I mistype the shard name in my shard.yml? I wont get a name mismatch error anymore, but a no versions available message (what?).

Please dont rename shards. But if you really must, please create a new repository. You can keep the history, but must delete all the version tags.

[straight-shoota]

Please dont rename shards. But if you really must, please create a new repository. You can keep the history, but must delete all the version tags.

I can follow the first part, but is deleting all version tags really the best solution? An alternative would be to update all existing tags to a commit which changes the name to the new one. That could probably work, but it's still tedious and somewhat breaks backwards compatibility because the commit hashes don't match anymore.

Considering the case there is a share with name A and it's no longer maintained, the maintained not reachable etc. If you want to continue its development as shard B (to make sure it's a different thing), you might still want to build on the history of shard A.

I'm not sure / haven't thought this entirely through, but keeping the old name for old releases seems somewhat reasonable.

[ysbaddaden]

How to draw the line between before and now? How can Shards know that it musn't consider older release tags?

[straight-shoota]

What's the exact purpose of this name validation anyway?

As far as I can see, it seems to be just additional verification to make sure there is no mistake. It shouldn't be strictly necessary to enforce. In general, the name of the dependency should match the shard name, but are there any issues when it doesn't in all releases? Obviously, the dependency name affects the available require paths. But that wouldn't matter when it only affects obsolete releases.

Maybe, as a simple rule, the resolver could just skip versions where the shard name doesn't match the dependency name. It would essentially be treated the same way as if there was no spec for this version. This seems to be quite similar to considering a spec with a different name as a spec for a different dependency. Thus a spec with different shard name would not be recognized as a valid version of the requested dependency.

[ysbaddaden]

No. The name must be validated. At least to catch name mismatches and avoid skipping all releases and selecting the HEAD refs to be installed (default behavior for shards without releases). Oops, that's a bug, and a silent one that can easily go unnoticed (nasty).

Maybe, as a simple rule, the resolver could just skip versions where the shard name doesn't match the dependency name.

Nope. See bug above. It would be impossible to distinguish a renamed shard without releases (install HEAD) from a name mismatch (error out).

We may make it work, but will likely introduce subtle edge case bugs (such as the obvious one above), and mostly complexify the detection, requiring patches for fixing the edge cases.

Please remember that Shards is a decentralized tool, and the repository itself is BOTH the source code AND the releases of a shard (one shard only).

[ysbaddaden]

Also: please prove me wrong, if you can fix it without increasing complexity and keep a green test suite —without tweaking tests to make 'em pass, of course— and proof that it shouldn't break anything, please go for it.

[straight-shoota]

At least to catch name mismatches and avoid skipping all releases and selecting the HEAD refs to be installed

Mismatches wouldn't happen if versions with different reported names are sorted out. I think skipping all releases and selecting HEAD would actually be expected. In the scenario that a shard was renamed (for whatever reason) from foo to bar, when the dependency name is bar and there is not yet a release tagged for bar, then HEAD should be used. If it reports foo as well, then it should fail.

It would be impossible to distinguish a renamed shard without releases (install HEAD) from a name mismatch (error out).

I can't see a problem with that: When at least one release exists (including HEAD) that reports the same name as the dependency name, it's a renamed shard. If there is no release reporting that name, it's a mismatch.

The basic implementation is quite trivial: https://github.com/crystal-lang/shards/compare/master...straight-shoota:feature/renamed-shard?expand=1 I don't see any obvious edge cases, but I'll look into it.

Theoretically, this mechanism could be abused for providing different shards from the same repository (but only allows disparate versions). This could technically be prevented by requiring HEAD to match the dependency name. But then you couldn't install older versions (with a different name) from the same repository.

[RX14]

I agree with @ysbaddaden - it's easy enough to delete the tags on the remote git repository and publish a new one under the new name with the same git refs

[bcardiff]

Although deleting tags is technically possible, on repositories with multiple collaborators it is harder to wipe them out completely. They come back on someone's push IME.

I value more the chance to recover from a unintended human error as stated in the OC without deleting tags or recreating the repo. And I see that that leads to been able to rename the shard. Since the repo name by itself is not important, the name of the shard should be able to change, and without breaking existing projects that have been using it as a dependency.