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

"." as a relative ports directory does not work #30942

Closed
vadi2 opened this issue Apr 18, 2023 · 9 comments
Closed

"." as a relative ports directory does not work #30942

vadi2 opened this issue Apr 18, 2023 · 9 comments
Assignees
Labels
category:question This issue is a question Stale

Comments

@vadi2
Copy link
Contributor

vadi2 commented Apr 18, 2023

Describe the bug
My port is located in the same folder as my vcpkg.json file and using "overlay-ports": ["."] as a relative directory was not recognised:

  error: while loading /home/runner/work/Mudlet/Mudlet/3rdparty/our-vcpkg-dependencies/./vcpkg.json:
  $ (a manifest): missing required field 'name' (an identifier)
  $ (a manifest): expected a versioning field (one of version, version-date, version-semver, or version-string)Failed to load port hunspell from /home/runner/work/Mudlet/Mudlet/3rdparty/our-vcpkg-dependencies/.

Environment

To Reproduce
Steps to reproduce the behavior:

  1. create a port directory
  2. create vcpkg.json as a sibling to the port directory
  3. specify "overlay-ports": ["."] for all ports in the current directory to be found

Expected behavior
Vcpkg finds the ports specified in the same directory as the vcpkg.json file is.

Additional context
I was able to work around it by specifying just the 1 port I have in the same directory as "./portname"

@FrankXie05
Copy link
Contributor

Using "." to represent the current directory can have different meanings in different operating systems and terminal environments.

To avoid confusion and errors, we just allow to use explicit relative or absolute paths instead of "." when specifying the path to the overlay ports directory.

Example:
https://github.com/microsoft/vcpkg-docs/blob/69039427fc5fb9e9310618d1e0765bd3b20fe7f0/vcpkg/reference/vcpkg-json.md#example-3

@FrankXie05 FrankXie05 added the category:question This issue is a question label Apr 18, 2023
@vadi2
Copy link
Contributor Author

vadi2 commented Apr 18, 2023

Using "." to represent the current directory can have different meanings in different operating systems and terminal environments.

Can you give examples? Pretty much everywhere I know "." means current directory, and since you already accept it in ./port-name I'm not grasping where the confusion could be.

@dg0yt
Copy link
Contributor

dg0yt commented Apr 19, 2023

IMO . is a valid, explicit, relative path.

@FrankXie05
Copy link
Contributor

Explain two points:

  1. Regarding your question:

I was able to work around it by specifying just the 1 port I have in the same directory as "./portname"

We are unable to determine what you passed in when using overlay-ports Is there a folder or port file after (representing the current directory of the relative path) (./my-ports/fmt or ./team-ports).

Therefore, a complete name must be provided.

  1. Regarding the issue with.:

Can you give examples? Pretty much everywhere I know "." means current directory, and since you already accept it in ./port-name I'm not grasping where the confusion could be.

Yes, usually in a computer's operating system,. represents the current directory.

However, there are still some slight differences or special uses between different operating systems.
We try to avoid encountering such situations as much as possible (Although there is a small chance of encountering it 🫤):

  • The use of path separators.

  • It can also represent script files in Linux

  • In certain special cases:
    Windows: Represents a hidden directory
    Linux: regular directory

@dg0yt
Copy link
Contributor

dg0yt commented Apr 20, 2023

In the context of directory paths, . is a relative path pointing to the current directory.
There might be good reason to exclude the current directory, no matter how it written. But I really don't get why you want to drag a clear, non-ambiguous fact into the land of uncertainty.

  • It can also represent script files in Linux

. does not "represent script files in Linux".
However, . is also a command in bash -- and also in powershell. But this is another context.
There is no policy to forbid command names (e.g. ls) for overlay port dir names.

  • In certain special cases:
    Windows: Represents a hidden directory.
    Linux: regular directory

I don't know if this is wrong or if it is meant in another way.
Windows uses attributes to mark hidden files. (But I don't know how it handles . internally.)
On Linux and similar systems, a dot at the beginning of a filename marks a hidden file/dir.
There is no policy to forbid hidden overlay port dirs (e.g. .overlays).

@BillyONeal
Copy link
Member

This is actually intended behavior but it is somewhat unfortunate. The issue is that overlay ports can refer to one of:

  1. A directory, containing other directories of port names, which are themselves overlay ports.
  2. The overlay port directory itself.

You are falling into case 2: the vcpkg.json that itself is attempting to declare overlay-ports: ., is itself being interpreted as the overlay port.

Put another way,

x/vcpkg.json # name = foo
x/a/vcpkg.json # name = a
x/b/vcpkg.json # name = b
x/c/vcpkg.json # name = c

setting --overlay-ports=x brings in the overlay port foo, not a, b, or c.

Unfortunately I think this means you need the overlays to be in a subdirectory.

@github-actions
Copy link

This is an automated message. Per our repo policy, stale issues get closed if there has been no activity in the past 28 days. The issue will be automatically closed in 14 days. If you wish to keep this issue open, please add a new comment.

@github-actions github-actions bot added the Stale label Jun 18, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 2, 2023
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this issue Oct 22, 2024
This work is part of resolving microsoft/vcpkg#30942 / https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597

When overlay directories from the config were added in microsoft#743 this added a condition where VcpkgPaths took responsibility to tack on the relative path, transforming those paths that come from the config into absolute paths. I did not realize at the time that this block was repeating machinery already present in IOverlayProvider family. The difference is that, there, it always assumed the prefix would be original_cwd.

As part of adding overlay-port-dirs, I needed to add the same kind of prefix handling as overlay-ports get, thus pointing this out to me. It doesn't make sense to keep two independent ways to do this, leaving two options:

* Move the prefix stapling into VcpkgPaths (as done in this PR)
* Move the prefix stapling related to configs down into OverlayPortProviderImpl

I chose to move into VcpkgPaths for several reasons:

* Plumbing the information about how to handle config paths correctly into OverlayPortProviderImpl would have added many many function parameters to large parts of the product that are only passing paths information around.
* The decision to chdir in VcpkgPaths is a consideration *it* makes, and OverlayPortProviderImpl would be just as happy if the relative paths were left alone. (And in fact I think the right behavior in the future would be to remove this harmful chdir but that is not proposed right now)
* Despite making VcpkgPaths do "more work" here, I actually argue that it makes the "VcpkgPaths big ball of mud" problem better, by removing parts of the code that need to consider original_cwd.
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this issue Oct 24, 2024
This work is part of resolving microsoft/vcpkg#30942 / https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597

When overlay directories from the config were added in microsoft#743 this added a condition where VcpkgPaths took responsibility to tack on the relative path, transforming those paths that come from the config into absolute paths. I did not realize at the time that this block was repeating machinery already present in IOverlayProvider family. The difference is that, there, it always assumed the prefix would be original_cwd.

As part of adding overlay-port-dirs, I needed to add the same kind of prefix handling as overlay-ports get, thus pointing this out to me. It doesn't make sense to keep two independent ways to do this, leaving two options:

* Move the prefix stapling into VcpkgPaths (as done in this PR)
* Move the prefix stapling related to configs down into OverlayPortProviderImpl

I chose to move into VcpkgPaths for several reasons:

* Plumbing the information about how to handle config paths correctly into OverlayPortProviderImpl would have added many many function parameters to large parts of the product that are only passing paths information around.
* The decision to chdir in VcpkgPaths is a consideration *it* makes, and OverlayPortProviderImpl would be just as happy if the relative paths were left alone. (And in fact I think the right behavior in the future would be to remove this harmful chdir but that is not proposed right now)
* Despite making VcpkgPaths do "more work" here, I actually argue that it makes the "VcpkgPaths big ball of mud" problem better, by removing parts of the code that need to consider original_cwd.
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this issue Oct 29, 2024
This work is part of resolving microsoft/vcpkg#30942 / https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597

When overlay directories from the config were added in microsoft#743 this added a condition where VcpkgPaths took responsibility to tack on the relative path, transforming those paths that come from the config into absolute paths. I did not realize at the time that this block was repeating machinery already present in IOverlayProvider family. The difference is that, there, it always assumed the prefix would be original_cwd.

As part of adding overlay-port-dirs, I needed to add the same kind of prefix handling as overlay-ports get, thus pointing this out to me. It doesn't make sense to keep two independent ways to do this, leaving two options:

* Move the prefix stapling into VcpkgPaths (as done in this PR)
* Move the prefix stapling related to configs down into OverlayPortProviderImpl

I chose to move into VcpkgPaths for several reasons:

* Plumbing the information about how to handle config paths correctly into OverlayPortProviderImpl would have added many many function parameters to large parts of the product that are only passing paths information around.
* The decision to chdir in VcpkgPaths is a consideration *it* makes, and OverlayPortProviderImpl would be just as happy if the relative paths were left alone. (And in fact I think the right behavior in the future would be to remove this harmful chdir but that is not proposed right now)
* Despite making VcpkgPaths do "more work" here, I actually argue that it makes the "VcpkgPaths big ball of mud" problem better, by removing parts of the code that need to consider original_cwd.
BillyONeal added a commit to BillyONeal/vcpkg-tool that referenced this issue Oct 31, 2024
This work is part of resolving microsoft/vcpkg#30942 / https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597

When overlay directories from the config were added in microsoft#743 this added a condition where VcpkgPaths took responsibility to tack on the relative path, transforming those paths that come from the config into absolute paths. I did not realize at the time that this block was repeating machinery already present in IOverlayProvider family. The difference is that, there, it always assumed the prefix would be original_cwd.

As part of adding overlay-port-dirs, I needed to add the same kind of prefix handling as overlay-ports get, thus pointing this out to me. It doesn't make sense to keep two independent ways to do this, leaving two options:

* Move the prefix stapling into VcpkgPaths (as done in this PR)
* Move the prefix stapling related to configs down into OverlayPortProviderImpl

I chose to move into VcpkgPaths for several reasons:

* Plumbing the information about how to handle config paths correctly into OverlayPortProviderImpl would have added many many function parameters to large parts of the product that are only passing paths information around.
* The decision to chdir in VcpkgPaths is a consideration *it* makes, and OverlayPortProviderImpl would be just as happy if the relative paths were left alone. (And in fact I think the right behavior in the future would be to remove this harmful chdir but that is not proposed right now)
* Despite making VcpkgPaths do "more work" here, I actually argue that it makes the "VcpkgPaths big ball of mud" problem better, by removing parts of the code that need to consider original_cwd.
BillyONeal added a commit to microsoft/vcpkg-tool that referenced this issue Nov 1, 2024
…1522)

This work is part of resolving microsoft/vcpkg#30942 / https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597

When overlay directories from the config were added in #743 this added a condition where VcpkgPaths took responsibility to tack on the relative path, transforming those paths that come from the config into absolute paths. I did not realize at the time that this block was repeating machinery already present in IOverlayProvider family. The difference is that, there, it always assumed the prefix would be original_cwd.

As part of adding overlay-port-dirs, I needed to add the same kind of prefix handling as overlay-ports get, thus pointing this out to me. It doesn't make sense to keep two independent ways to do this, leaving two options:

* Move the prefix stapling into VcpkgPaths (as done in this PR)
* Move the prefix stapling related to configs down into OverlayPortProviderImpl

I chose to move into VcpkgPaths for several reasons:

* Plumbing the information about how to handle config paths correctly into OverlayPortProviderImpl would have added many many function parameters to large parts of the product that are only passing paths information around.
* The decision to chdir in VcpkgPaths is a consideration *it* makes, and OverlayPortProviderImpl would be just as happy if the relative paths were left alone. (And in fact I think the right behavior in the future would be to remove this harmful chdir but that is not proposed right now)
* Despite making VcpkgPaths do "more work" here, I actually argue that it makes the "VcpkgPaths big ball of mud" problem better, by removing parts of the code that need to consider original_cwd.
@BillyONeal BillyONeal reopened this Nov 2, 2024
@BillyONeal
Copy link
Member

microsoft/vcpkg-tool#1534 adds a new "overlay-port-dirs" option, which never considers the provided directory itself to the port, meaning "overlay-port-dirs": ["."] will have the effect you wanted as soon as there is a tool release with this change.

@BillyONeal
Copy link
Member

After feedback there seems to be strong opposition to allowing "." here. At least we're adding a warning to prevent the going in circles.

@BillyONeal BillyONeal closed this as not planned Won't fix, can't repro, duplicate, stale Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:question This issue is a question Stale
Projects
None yet
Development

No branches or pull requests

4 participants