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

Fix HL2SDK manifests lookup #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akxcv
Copy link

@akxcv akxcv commented Nov 20, 2023

POV: tried to build a project copied from s2_sample_mm project

Issue: it tried to look up manifests in the project folder, no matter what's specified in --hl2sdk-manifests.

Traceback (most recent call last):
  File "C:\Users\ak\AppData\Local\Programs\Python\Python310\lib\site-packages\ambuild2\frontend\v2_2\prep.py", line 156, in Configure
    if not cm.generate(options.generator):
  File "C:\Users\ak\AppData\Local\Programs\Python\Python310\lib\site-packages\ambuild2\frontend\context_manager.py", line 93, in generate
    self.parseBuildScripts()
  File "C:\Users\ak\AppData\Local\Programs\Python\Python310\lib\site-packages\ambuild2\frontend\v2_2\context_manager.py", line 50, in parseBuildScripts
    self.execContext(cx)
  File "C:\Users\ak\AppData\Local\Programs\Python\Python310\lib\site-packages\ambuild2\frontend\v2_2\context_manager.py", line 148, in execContext
    exec(code, scriptGlobals)
  File "D:\dev\csgo\whitelist\AMBuildScript", line 296, in <module>
    MMSPlugin.detectSDKs()
  File "D:\dev\csgo\whitelist\AMBuildScript", line 114, in detectSDKs
    SdkHelpers.findSdks(builder, self.all_targets, sdk_list)
  File "D:\dev\csgo\metamod-source\hl2sdk-manifests\SdkHelpers.ambuild", line 23, in findSdks
    for sdk_name, sdk in SdkHelpers.getSdks(builder):
  File "D:\dev\csgo\metamod-source\hl2sdk-manifests\SdkHelpers.ambuild", line 126, in getSdks
    for sdk_manifest in os.listdir(sdk_manifest_dir):
FileNotFoundError: [WinError 3] The system cannot find the path specified: 'D:\\dev\\csgo\\whitelist\\hl2sdk-manifests\\manifests'
Configure failed: [WinError 3] The system cannot find the path specified: 'D:\\dev\\csgo\\whitelist\\hl2sdk-manifests\\manifests'

@psychonic psychonic self-requested a review November 20, 2023 21:58
@psychonic psychonic self-requested a review December 17, 2023 18:01
Copy link
Member

@psychonic psychonic left a comment

Choose a reason for hiding this comment

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

Sorry for the delay on this. At a glance it seemed fine, but we can't rely on there being an hl2sdk-manifests option because that is up to the consumer of hl2sdk-manifests. For example, Metamod:Source doesn't implement an option for it because it uses an in-tree submodule, giving it a known path without need for configuration.

I suggest that approach be used for plugins as well, even though there's not a great way for the s2 sample plugin in the MM:S tree to do so with the way it is nested. If there is confusion on that side, we can move the sample to its own repo as a more full example.

As for working around that issue on the side of this repo, this change could instead look up the path of the current file (SdkHelpers.ambuild) and deduce the hl2sdk-manifests path from that, as it lives under that directory.

@akxcv
Copy link
Author

akxcv commented Mar 16, 2024

Sorry for the delay on this. At a glance it seemed fine, but we can't rely on there being an hl2sdk-manifests option because that is up to the consumer of hl2sdk-manifests. For example, Metamod:Source doesn't implement an option for it because it uses an in-tree submodule, giving it a known path without need for configuration.

I suggest that approach be used for plugins as well, even though there's not a great way for the s2 sample plugin in the MM:S tree to do so with the way it is nested. If there is confusion on that side, we can move the sample to its own repo as a more full example.

As for working around that issue on the side of this repo, this change could instead look up the path of the current file (SdkHelpers.ambuild) and deduce the hl2sdk-manifests path from that, as it lives under that directory.

What do you think about respecting the --hl2sdk-manifests option if it is provided, and falling back to the default path in case this option is missing? To me, this behaviour seems quite straightforward, and we could explicitly document that option for extra clarity.

Though, I admit that it feels wrong that this repo seems to be in charge of looking itself up.

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

Successfully merging this pull request may close these issues.

None yet

2 participants