Update pubspec.lock/package_config.json after running "pub get" as a workaround #5760
Conversation
…workaround ... for `pub` not updating these. This fixes #5549 but once dart-lang/pub#4672 is fixed we should change the upper bound so we stop doing it for newer SDKs, because it's a bit hacky.
|
@codex review |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a workaround for a Dart/Pub issue where pubspec.lock and package_config.json files are not properly updated after running pub get. The solution touches these files to update their modification times for Dart SDK versions 3.7.0 and above.
Key Changes:
- Added a
touchFileutility function to update file modification times - Introduced version detection for when the workaround is needed
- Modified
pub getcommand execution to touch relevant files after completion
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/shared/utils/fs.ts | Adds touchFile utility function to update file modification times |
| src/shared/capabilities/dart.ts | Adds version capability check for the pub get timestamp workaround |
| src/extension/commands/packages.ts | Implements the workaround by touching pub files after running pub get |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/extension/commands/packages.ts
Outdated
|
|
||
| // Touch the files to update their modification times. | ||
| // This is a workaround for https://github.com/Dart-Code/Dart-Code/issues/5549. | ||
| if (result && this.dartCapabilities.requiresTouchAfterPubGet) |
There was a problem hiding this comment.
The condition only checks if result is truthy, but doesn't verify if the pub get operation was successful. If result.exitCode indicates failure, touching the files could be misleading. Consider checking result.exitCode === 0 to ensure the operation completed successfully before touching the files.
| if (result && this.dartCapabilities.requiresTouchAfterPubGet) | |
| if (result && result.exitCode === 0 && this.dartCapabilities.requiresTouchAfterPubGet) |
There was a problem hiding this comment.
Code Review
This pull request introduces a workaround for an issue where pub get may not update file modification timestamps, which can affect file watchers. The change involves 'touching' pubspec.lock and package_config.json after pub get completes. This behavior is correctly controlled by a new capability flag, requiresTouchAfterPubGet, which restricts it to specific Dart SDK versions. The implementation is clean and the approach is sound. I have one suggestion to improve the robustness of the new touchFile utility function.
| } catch { | ||
| // Silently ignore errors (file doesn't exist, permission issues). | ||
| } |
There was a problem hiding this comment.
While silently ignoring expected errors is fine, using a broad, empty catch block can be risky as it might hide unexpected issues (for example, a TypeError from a typo in a function name). It would be safer to only ignore specific, expected error codes and log any others. This will aid future debugging if something unexpected goes wrong.
Other functions in this file, like tryDeleteFile and projectReferencesFlutter, use console.warn for logging, so it would be consistent to do so here as well.
} catch (e: any) {
// Silently ignore expected errors like permissions or file-not-found
// (it may have been deleted between our check and use), but log others
// to aid debugging.
if (e.code !== "ENOENT" && e.code !== "EPERM" && e.code !== "EACCES") {
console.warn(`Failed to touch file ${filePath}: ${e}`);
}
}There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
... for
pubnot updating these.This fixes #5549 but once dart-lang/pub#4672 is fixed we should change the upper bound so we stop doing it for newer SDKs, because it's a bit hacky.