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

pub deps --json #2896

Merged
merged 7 commits into from
Mar 16, 2021
Merged

pub deps --json #2896

merged 7 commits into from
Mar 16, 2021

Conversation

sigurdm
Copy link
Contributor

@sigurdm sigurdm commented Mar 2, 2021

@google-cla google-cla bot added the cla: yes label Mar 2, 2021
@sigurdm sigurdm requested a review from jonasfj March 2, 2021 16:07
lib/src/command/deps.dart Outdated Show resolved Hide resolved
@sigurdm sigurdm changed the title Deps json pub deps --json Mar 4, 2021
@sigurdm sigurdm requested a review from jonasfj March 8, 2021 12:13
@@ -67,23 +72,59 @@ class DepsCommand extends PubCommand {
if (argResults['executables']) {
_outputExecutables();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have mixed feelings about this.

We could choose to output different JSON output when either --dev and/or --executable is given, or we could just ignore (or forbid) these flags when outputting JSON, and always output the same thing.

Since a JSON output could contain a property "relation": "direct | dev | transitive | transitive-dev" or something like that, and we leave it up to the reader to parse it.

Didn't we do this for dart pub outdated --json ?

@sigurdm
Copy link
Contributor Author

sigurdm commented Mar 15, 2021

cc: @DanTup do you have any comments about the json output, seen from the perspective of dart-code?

@DanTup
Copy link

DanTup commented Mar 15, 2021

@sigmundch I haven't looked properly yet, but I'll aim to take a look this afternoon and let you know!

@DanTup
Copy link

DanTup commented Mar 15, 2021

@sigmundch looks great! It should make both Dart-Code/Dart-Code#3095 and Dart-Code/Dart-Code#3202 very easy to add. Thanks! :-)

@sigurdm sigurdm merged commit 056a8c9 into dart-lang:master Mar 16, 2021
@DanTup
Copy link

DanTup commented May 25, 2021

Is it intended that package_config.json is modified when running this command? It causes a loop for me right now - we refresh the packages tree when that file changes, but we then call this command to render the tree, so it ends up constantly refreshing. The timestamp inside the JSON doesn't change, but the file modified date changes (and the file watchers fire).

I can work around it, it's just a little clunky (the event fired to tell VS Code to refresh the tree is somewhat disconnected from the code that then later calls the command when asked for the tree nodes) so I thought it was worth checking.

@jonasfj
Copy link
Member

jonasfj commented May 27, 2021

@DanTup please file an issue if you can reproduce it.

[myapp]$dart pub get
Resolving dependencies... 
Got dependencies!
[myapp]$stat  .dart_tool/package_config.json 
  File: .dart_tool/package_config.json
  Size: 650       	Blocks: 8          IO Block: 4096   regular file
Device: fe01h/65025d	Inode: 3950172     Links: 1
Access: (0644/-rw-r--r--)  Uid: (591453/ jonasfj)   Gid: (89939/primarygroup)
Access: 2021-05-27 11:35:56.182021029 +0200
Modify: 2021-05-27 11:42:16.708804560 +0200
Change: 2021-05-27 11:42:16.708804560 +0200
 Birth: 2021-05-27 11:35:56.182021029 +0200
[myapp]$dart pub deps --json > /dev/null
[myapp]$stat  .dart_tool/package_config.json 
  File: .dart_tool/package_config.json
  Size: 650       	Blocks: 8          IO Block: 4096   regular file
Device: fe01h/65025d	Inode: 3950172     Links: 1
Access: (0644/-rw-r--r--)  Uid: (591453/ jonasfj)   Gid: (89939/primarygroup)
Access: 2021-05-27 11:35:56.182021029 +0200
Modify: 2021-05-27 11:42:16.708804560 +0200
Change: 2021-05-27 11:42:16.708804560 +0200
 Birth: 2021-05-27 11:35:56.182021029 +0200

I couldn't trivially reproduce it. dart pub get will update the mtime, even if there is no changes... we could consider changing that.

@DanTup
Copy link

DanTup commented May 27, 2021

Hmm, I can't repro in one project but can in another:

danny@Dannys-MacBook-Pro dart_sample % stat  .dart_tool/package_config.json                   
16777221 47509992 -rw-r--r-- 1 danny staff 0 10044 "May 27 11:43:48 2021" "May 27 11:43:48 2021" "May 27 11:43:48 2021" "Feb 11 13:03:20 2021" 4096 24 0 .dart_tool/package_config.json

danny@Dannys-MacBook-Pro dart_sample % ~/Dev/Dart\ SDKs/nightly-2021-05-25/bin/pub deps --json > /dev/null 

danny@Dannys-MacBook-Pro dart_sample % stat  .dart_tool/package_config.json                               
16777221 47509992 -rw-r--r-- 1 danny staff 0 10044 "May 27 11:44:01 2021" "May 27 11:44:01 2021" "May 27 11:44:01 2021" "Feb 11 13:03:20 2021" 4096 24 0 .dart_tool/package_config.json

I'll see if I can figure out the difference and file an issue. Thanks!

@DanTup
Copy link

DanTup commented May 27, 2021

dart pub get will update the mtime, even if there is no changes... we could consider changing that.

That's not a problem for me here - the issue was just with deps because we call it in response to the modification, and if it triggers another, it loops. Although pub get would trigger the tree to refresh, the tree would not call back out to pub get so there would be no loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants