dart-lang / pub

The pub command line tool
https://dart.dev/tools/pub/cmd
BSD 3-Clause "New" or "Revised" License
1.04k stars 228 forks source link

pub global activate via path not working unless `.dart_tool/pub/bin/<executable>` is deleted #4409

Open btrautmann opened 3 weeks ago

btrautmann commented 3 weeks ago
#### General info

- Dart 3.5.0 (stable) (Tue Jul 30 02:17:59 2024 -0700) on "macos_arm64"
- on macos / Version 14.7 (Build 23H124)
- locale is en-US

#### Project info

- sdk constraint: '^3.5.0'
- dependencies: 
- dev_dependencies: lints, test

#### Process info

|  Memory |  CPU | Elapsed time | Command line                                                                    |
| ------: | ---: | -----------: | ------------------------------------------------------------------------------- |
|   23 MB | 0.0% |     22:57:26 | dart devtools --machine --allow-embedding --dtd-uri ws:<path>/SLOui8f56EyIjsj7  |
| 2278 MB | 0.0% |     22:57:27 | dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.96.0 |
|  464 MB | 0.0% |        03:18 | dart language-server --protocol=lsp --client-id=VS-Code --client-version=3.96.0 |
|   22 MB | 0.0% |     22:57:27 | dart tooling-daemon --machine                                                   |
|   82 MB | 0.0% |        03:18 | dart tooling-daemon --machine                                                   |
|   49 MB | 0.3% |     22:57:27 | flutter_tools.snapshot daemon                                                   |

The Issue

I started noticing a few weeks ago that dart pub global activate --source="path" <path> stopped working. To reproduce, you can:

  1. use a project created via dart create -t console <name>
  2. add the following to the pubspec.yaml
    executables:
    name: name
  3. activate the executable globally via dart pub global activate --source="path" <path/to/root/of/project>
  4. Run the executable via name and note result
  5. Change the logic of the executable (in the case of the dart create -t console default project, update calculate() to return a different integer).
  6. Repeat step 3-4, noting that the result has not changed.
  7. Delete the .dart_tool/pub/bin/<name> directory
  8. Repeat steps 3-4, noting that the result does change this time.

Here is a sample repo you can clone and use to do the above.

Expected results

Prior to this bug, re-running dart pub global activate --source="path" would correctly update the activated executable such that the new logic would be respected.

dart-github-bot commented 3 weeks ago

Summary: The user reports that dart pub global activate --source="path" no longer updates activated executables when the executable's logic is changed. Deleting the .dart_tool/pub/bin/<executable> directory before activating the executable again resolves the issue.

btrautmann commented 3 weeks ago

Summary: The user reports that dart pub global activate --source="path" no longer updates activated executables when the executable's logic is changed. Deleting the .dart_tool/pub/bin/<executable> directory before activating the executable again resolves the issue.

Good bot

btrautmann commented 1 week ago

@bkonyi can you LMK if this fell through the cracks? This is a pretty annoying bug to run into, particularly in cases like self-hosted github actions runners where state is not wiped between runs and so implementation hangs around until you remember this is a thing. I imagine there are others scratching their heads as to why pub global activate is not working. 👀

bkonyi commented 1 week ago

Yeah, I'm not sure if the pub team is subscribed to the dart-cli-pub label, so I'll just cc @sigurdm and @jonasfj to make sure they're aware.

sigurdm commented 1 week ago

Yeah - this is a bug. We are using snapshots, but we don't get to invalidate them (as dart run does).

sigurdm commented 1 week ago

Perhaps best would be to have the binstub for a path-activated package just call dart path/to/package/bin/executable.dart instead of precompiling.

Alternatively the binstub needs to recompile (like we do in dart run).

btrautmann commented 1 week ago

Yeah - this is a bug. We are using snapshots, but we don't get to invalidate them (as dart run does).

Just curious: This at one point did work the way I expected; did something change?

sigurdm commented 1 week ago

A bisect shows it was introduced by a3ee209988af39c1d4f3c67b4910c317047356ca #3586

I can see that the claim "Always generate the full logic in the binstub (it will check for snapshot presence anyways)" is missing the point. There is no check if the code is updated relative to the snapshot.

sigurdm commented 1 week ago

Sorry about that. We should probably revert to having the binstubs just invoke dart global run <dart-file>.dart instead of trying to use a snapshot.