invertase / melos

๐ŸŒ‹ A tool for managing Dart projects with multiple packages. With IntelliJ and Vscode IDE support. Supports automated versioning, changelogs & publishing via Conventional Commits.
https://melos.invertase.dev/~melos-latest
Apache License 2.0
1.08k stars 193 forks source link

fix: fix git dependencies comparison on bootstrap #659

Open mateusfccp opened 4 months ago

mateusfccp commented 4 months ago

Description

Fixes comparison check of GitReference by also comparing the path.

Fixes #658.

Type of Change

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

mateusfccp commented 4 months ago

Can you add a very simple test that verifies this, so that we don't get a regression later?

Sure, but it will take a little longer, as I'm not very acquainted with this repository's tests.

spydon commented 4 months ago

Sure, but it will take a little longer, as I'm not very acquainted with this repository's tests.

Super! You've probably already found it, but if not, you might be able to get some inspiration from the existing bootstrap tests (where the new test should live too): https://github.com/invertase/melos/blob/main/packages/melos/test/commands/bootstrap_test.dart

mateusfccp commented 4 months ago

Hey, @spydon.

Do you have any idea on how can I test git cloning with the tests suit? For path, for instance, we have createTestTempDir, so that we can create temporary files that we use for testing.

For git, however, we have nothing. I don't fell like it would be advisable to test with a real repository, so I think we should fake this process, but I am not aware how we could do it.

Currently, I'm using this fake object for a git reference:

final pkg = await createProject(
  workspaceDir,
  const PubSpec(
    name: 'package',
    dependencies: {
      'git': GitReference(
        'url',
        'reference',
        'path',
      ),
    },
  ),
);

However, as I'm not faking the git repository, melos test tries to clone from bare repository /Users/<myuser>/.puro/shared/pub_cache/git/cache/url-0a6bd4f3b43c4001eceeb17e5638198fa98773db and fails miserably.

spydon commented 4 months ago

Hey again @mateusfccp!

Do you have any idea on how can I test git cloning with the tests suit? For path, for instance, we have createTestTempDir, so that we can create temporary files that we use for testing.

Can't you do the same, but with a temporary local git repository? You should be able to use a git file path url (not a path dependency) to that temporary repository.

mateusfccp commented 4 months ago

@spydon

Can't you do the same, but with a temporary local git repository?

I actually didn't know we could do something like this. It worked!

I'm not sure if this is the most straightforward way to test this, but it works. Let me know if I can improve it somehow.

mateusfccp commented 4 months ago

It seems it failed on Windows and Linux, although it worked on Mac (the platform I'm working with).

The error seems to indicate that the path is not a git repository, even tho I initialized it and made a commit.

Unfortunately, I have no Linux or Window machine to debug the problem here... Do you have any idea what it could be?

spydon commented 3 months ago

It seems it failed on Windows and Linux, although it worked on Mac (the platform I'm working with).

The error seems to indicate that the path is not a git repository, even tho I initialized it and made a commit.

Unfortunately, I have no Linux or Window machine to debug the problem here... Do you have any idea what it could be?

Sorry, I though I replied after reading it a few days ago! Seems like you managed to sort it out?

mateusfccp commented 3 months ago

It seems it failed on Windows and Linux, although it worked on Mac (the platform I'm working with). The error seems to indicate that the path is not a git repository, even tho I initialized it and made a commit. Unfortunately, I have no Linux or Window machine to debug the problem here... Do you have any idea what it could be?

Sorry, I though I replied after reading it a few days ago! Seems like you managed to sort it out?

No, not really. I just rebased the PR...

I searched for potential differences in git between platforms, but didn't find anything relevant, so still in the darkness.

Edit: I just asked for a friend to try to run the test in his Windows machine and the test passed.

image

Maybe it's some difference in the CI environment? Could be permission related?

spydon commented 3 months ago

@mateusfccp just realized you probably can't see the CI logs? Here's what it says:

   Resolving dependencies...
  -Because git_references depends on dependency from git which doesn't exist (Could not find git ref 'HEAD' (usage: git rev-list [<options>] <commit>... [--] [<path>...]
  -  limiting output:
...
  -    --bisect-all)), version solving failed.

  BootstrapException: Failed to install.: git_references at /home/runner/work/melos/melos/packages/melos/.dart_tool/GKWRBX/packages/git_references.
  package:melos/src/commands/bootstrap.dart 243:7  _BootstrapMixin._runPubGetForPackage
mateusfccp commented 3 months ago

@mateusfccp just realized you probably can't see the CI logs? Here's what it says:

   Resolving dependencies...
  -Because git_references depends on dependency from git which doesn't exist (Could not find git ref 'HEAD' (usage: git rev-list [<options>] <commit>... [--] [<path>...]
  -  limiting output:
...
  -    --bisect-all)), version solving failed.

  BootstrapException: Failed to install.: git_references at /home/runner/work/melos/melos/packages/melos/.dart_tool/GKWRBX/packages/git_references.
  package:melos/src/commands/bootstrap.dart 243:7  _BootstrapMixin._runPubGetForPackage

Yes, I can see them. Like I said, this error seems to mean that the git repository was not created correctly or the commit.

I did pass through this error while writing the test, because at first I was only creating the git repository with git init, but I realized that to have a valid reference you have to have at least one commit, this is why I added the initial commit to the test.

This is why I think either the git initialization or the commit is failing in some weird way.