Add command `lint-port` #720

autoantwort · 2022-09-28T17:32:39Z

Adds command x-lint-port that checks for common port issues and the usage of deprecated functions. The command is also able to fix the most of these issues.

Thomas1664

I know that this is still a draft.

In general you should be careful with adding const to local variables because this might avoid moving in some cases

src/vcpkg/portlint.cpp

Thomas1664 · 2022-09-28T19:51:25Z

src/vcpkg/portlint.cpp

+namespace vcpkg::Lint
+{
+
+    constexpr StringLiteral VERSION_RELAXED = "version";


Suggested change

constexpr StringLiteral VERSION_RELAXED = "version";

static constexpr StringLiteral VERSION_RELAXED = "version";

The others as well.

Afaic https://stackoverflow.com/questions/59307723/static-constexpr-global-variable

src/vcpkg/commands.lint-port.cpp

src/vcpkg/portlint.cpp

Thomas1664 · 2022-09-29T15:22:41Z

In general, it might be possible to parallelize linting as well if there weren't any calls to check_exit() and alike.

autoantwort · 2022-09-29T15:28:34Z

In general, it might be possible to parallelize linting as well if there weren't any calls to check_exit() and alike.

Linting all ports currently takes 140 ms. I don't thing there is any need for performance improvements.

# Conflicts: # src/vcpkg/base/messages.cpp

# Conflicts: # locales/messages.en.json

vicroms

Some parts of this PR can be made unit-testable with a few tweaks.

src/vcpkg/portlint.cpp

# Conflicts: # azure-pipelines/end-to-end-tests-dir/format-manifest.ps1 # azure-pipelines/end-to-end-tests-dir/versions.ps1

@vicroms

Suggestions from @vicroms

autoantwort · 2022-11-27T03:58:03Z

@vicroms I have added unit tests

# Conflicts: # src/vcpkg/commands.add-version.cpp

reitowo · 2023-01-29T13:57:52Z

I think we can do both, the post build check can warn user, and we can expand this flag to something like --x-lint-port --fix-usage to automatically fix all ports usage issue and actually eliminate the issue.

autoantwort · 2023-01-29T14:00:15Z

I think you don't need a new flag for that. Simply use the --fix flag. Do you implement this and create a PR against my branch?

reitowo · 2023-01-29T14:00:58Z

Yes, I will create a PR against this branch once I tested.

reitowo · 2023-01-31T09:39:14Z

People use different ways to install usage, should vcpkg provide a unified vcpkg_install_usage?

autoantwort · 2023-01-31T12:30:54Z

I think the current situation is fine.

# Conflicts: # locales/messages.json # src/vcpkg/base/messages.cpp

# Conflicts: # locales/messages.json

talregev · 2023-03-08T09:09:00Z

@autoantwort Any news from your linter?

autoantwort · 2023-03-08T09:38:27Z

No, I am done and wait for review from @vicroms

talregev · 2023-03-13T19:48:29Z

@vicroms @Neumann-A @Thomas1664
Any news of review this PR?

Thomas1664

My earlier comment to add static to some string literals is still unfixed but like my new comment not a big deal.
I also noticed the use of std::string::size_type instead of std::size_t or size_t which is also not a problem.

src/vcpkg/portlint.cpp

include/vcpkg/portlint.h

talregev · 2023-03-14T09:41:54Z

@autoantwort Are you going to fix the conflicts?

autoantwort · 2023-03-14T09:47:31Z

I can fix them, but they don't prevent this PR from being reviewed.

# Conflicts: # include/vcpkg/base/messages.h # src/vcpkg/base/messages.cpp

talregev · 2023-03-14T15:39:37Z

@Thomas1664 this PR is ready for review by @vicroms from your side?

Thomas1664 · 2023-03-16T10:30:52Z

@Thomas1664 this PR is ready for review by @vicroms from your side?

The vcpkg team reviews PRs independently from community feedback such as my review comments.

# Conflicts: # include/vcpkg/base/messages.h # src/vcpkg/base/messages.cpp

# Conflicts: # src/vcpkg/commands.add-version.cpp

# Conflicts: # src/vcpkg/commands.cpp

talregev · 2023-05-11T01:38:39Z

Any news?

BillyONeal · 2023-06-12T19:31:35Z

Any news?

I think this keeps getting skipped over because we need to sell a concrete proposal of what this command actually does to our PM team and management, and we aren't getting time to reverse engineer what that looks like from this PR in order to do so.

talregev · 2023-11-23T12:43:47Z

Any news?

I think this keeps getting skipped over because we need to sell a concrete proposal of what this command actually does to our PM team and management, and we aren't getting time to reverse engineer what that looks like from this PR in order to do so.

All you need is time?

BillyONeal · 2023-11-28T00:19:47Z

All you need is time?

We need like a spec of what the command is supposed to do

Add command lint-port

9d0b2dd

Thomas1664 reviewed Sep 28, 2022

View reviewed changes

CR

c5d5557

autoantwort added 2 commits September 30, 2022 13:31

fix bug

32ef8e0

Adopt tests

689e47d

autoantwort mentioned this pull request Oct 5, 2022

[many-ports] Don't use deprecated functions (autogenerated) microsoft/vcpkg#26981

Merged

autoantwort added 10 commits October 25, 2022 14:06

Merge branch 'main' into feature/lint-port

7a5a5bd

# Conflicts: # src/vcpkg/base/messages.cpp

Merge branch 'main' into feature/lint-port

4cae60d

# Conflicts: # locales/messages.en.json

Fix warning

bfe12fe

Fix test

1bad290

Fix e2e tests

d3668f3

Fix windows

c48fb10

Ignore more wrong ports

23a7459

Merge branch 'main' into feature/lint-port

6795b99

Merge branch 'main' into feature/lint-port

a890651

Apply format diff

b0b4495

vicroms requested changes Nov 13, 2022

View reviewed changes

src/vcpkg/portlint.cpp Outdated Show resolved Hide resolved

src/vcpkg/portlint.cpp Show resolved Hide resolved

src/vcpkg/portlint.cpp Outdated Show resolved Hide resolved

autoantwort added 5 commits November 25, 2022 18:36

Merge branch 'main' into feature/lint-port

38b2a69

# Conflicts: # azure-pipelines/end-to-end-tests-dir/format-manifest.ps1 # azure-pipelines/end-to-end-tests-dir/versions.ps1

Make code more unit testable

686a36f

Suggestions from @vicroms

Add helpers to MessageSink

04ba447

Apply code review and add tests

ea500f5

Replace usages of vcpkg_extract_source_archive_ex

8f56f8e

autoantwort marked this pull request as ready for review November 27, 2022 03:57

autoantwort added 4 commits November 27, 2022 05:34

format diff

9e85bdd

Also replace REF with SOURCE_BASE in vcpkg_extract_source_archive

a8b28ea

Fix Bug

7f2c55a

Merge branch 'main' into feature/lint-port

4e17669

# Conflicts: # src/vcpkg/commands.add-version.cpp

Fix command name

68e6756

autoantwort added 2 commits February 9, 2023 15:09

Merge branch 'main' into feature/lint-port

a6f2608

# Conflicts: # locales/messages.json # src/vcpkg/base/messages.cpp

Merge branch 'main' into feature/lint-port

53df38b

# Conflicts: # locales/messages.json

Thomas1664 reviewed Mar 14, 2023

View reviewed changes

src/vcpkg/portlint.cpp Show resolved Hide resolved

include/vcpkg/portlint.h Show resolved Hide resolved

autoantwort added 2 commits March 14, 2023 10:53

Merge branch 'main' into feature/lint-port

ee0fda1

# Conflicts: # include/vcpkg/base/messages.h # src/vcpkg/base/messages.cpp

Improve performance

83b13a9

autoantwort added 6 commits March 19, 2023 03:10

Merge branch 'main' into feature/lint-port

89208e9

# Conflicts: # include/vcpkg/base/messages.h # src/vcpkg/base/messages.cpp

Merge branch 'main' into feature/lint-port

0c5c4f1

# Conflicts: # src/vcpkg/commands.add-version.cpp

Add missing include

4cdcb30

Use right command arguments

14f1b1f

Merge branch 'main' into feature/lint-port

f596c5a

# Conflicts: # src/vcpkg/commands.cpp

Merge branch 'main' into feature/lint-port

53e95f4

BillyONeal mentioned this pull request Oct 19, 2023

[review bot] Add comment to the PR instead of printing something to console that nobody reads microsoft/vcpkg#34549

Draft

autoantwort marked this pull request as draft November 28, 2023 00:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add command `lint-port` #720

Add command `lint-port` #720

autoantwort commented Sep 28, 2022 •

edited

Loading

Thomas1664 left a comment

Thomas1664 Sep 28, 2022

autoantwort Mar 14, 2023

Thomas1664 commented Sep 29, 2022

autoantwort commented Sep 29, 2022 •

edited

Loading

vicroms left a comment

autoantwort commented Nov 27, 2022

reitowo commented Jan 29, 2023

autoantwort commented Jan 29, 2023

reitowo commented Jan 29, 2023

reitowo commented Jan 31, 2023

autoantwort commented Jan 31, 2023

talregev commented Mar 8, 2023

autoantwort commented Mar 8, 2023

talregev commented Mar 13, 2023

Thomas1664 left a comment

talregev commented Mar 14, 2023

autoantwort commented Mar 14, 2023

talregev commented Mar 14, 2023

Thomas1664 commented Mar 16, 2023

talregev commented May 11, 2023

BillyONeal commented Jun 12, 2023

talregev commented Nov 23, 2023

BillyONeal commented Nov 28, 2023

	constexpr StringLiteral VERSION_RELAXED = "version";
	static constexpr StringLiteral VERSION_RELAXED = "version";

Add command lint-port #720

Are you sure you want to change the base?

Add command lint-port #720

Conversation

autoantwort commented Sep 28, 2022 • edited Loading

Thomas1664 left a comment

Choose a reason for hiding this comment

Thomas1664 Sep 28, 2022

Choose a reason for hiding this comment

autoantwort Mar 14, 2023

Choose a reason for hiding this comment

Thomas1664 commented Sep 29, 2022

autoantwort commented Sep 29, 2022 • edited Loading

vicroms left a comment

Choose a reason for hiding this comment

autoantwort commented Nov 27, 2022

reitowo commented Jan 29, 2023

autoantwort commented Jan 29, 2023

reitowo commented Jan 29, 2023

reitowo commented Jan 31, 2023

autoantwort commented Jan 31, 2023

talregev commented Mar 8, 2023

autoantwort commented Mar 8, 2023

talregev commented Mar 13, 2023

Thomas1664 left a comment

Choose a reason for hiding this comment

talregev commented Mar 14, 2023

autoantwort commented Mar 14, 2023

talregev commented Mar 14, 2023

Thomas1664 commented Mar 16, 2023

talregev commented May 11, 2023

BillyONeal commented Jun 12, 2023

talregev commented Nov 23, 2023

BillyONeal commented Nov 28, 2023

Add command `lint-port` #720

Add command `lint-port` #720

autoantwort commented Sep 28, 2022 •

edited

Loading

autoantwort commented Sep 29, 2022 •

edited

Loading