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

buildkite test failure #239

Open
dkorpel opened this issue Apr 18, 2023 · 8 comments · Fixed by dlang/dmd#15116
Open

buildkite test failure #239

dkorpel opened this issue Apr 18, 2023 · 8 comments · Fixed by dlang/dmd#15116

Comments

@dkorpel
Copy link
Contributor

dkorpel commented Apr 18, 2023

Buildkite on dmd and phobos fails to build ddox since recently:

https://buildkite.com/dlang/dmd/builds/31032#018792d2-86b8-4730-b9de-f90a0d4d7ccb

Compiling Diet HTML template home.dt...
Compiling Diet HTML template download.dt...
Compiling Diet HTML template error.dt...
Compiling Diet HTML template about.dt...
Compiling Diet HTML template contact.dt...
Compiling Diet HTML template community.dt...
Compiling Diet HTML template impressum.dt...
Compiling Diet HTML template features.dt...
Compiling Diet HTML template docs.dt...
Compiling Diet HTML template developer.dt...
Compiling Diet HTML template styleguide.dt...
Compiling Diet HTML template templates.dt...
Compiling Diet HTML template tutorials.dt...
     Linking vibed-org
             Copying files for rs-bootstrap...
             Copying files for diskuto...
+ '[' 1 -eq 1 ']'
+ exit 1
🚨 Error: The command exited with status 1
user command error: exit status 1

How should this log be interpreted?

@WebFreak001
Copy link
Member

probably:

+ echo 'FAILED: HTML generation test failed: tests/issue196_double_auto'
+ failure=1

and in the end it tests if failure == 1, in which case it exits with exit code 1 to indicate failure

Seems like tests are just broken here?

@s-ludwig
Copy link
Member

Looks like scope is now inferred for a function parameter, which changes the documentation accordingly.

- &nbsp;&nbsp;<span class="typ">int</span> <span class="pln">a</span><span class="pun">,</span>
+ &nbsp;&nbsp;<span class="kwd">scope </span><span class="typ">int</span> <span class="pln">a</span><span class="pun">,</span>

Maybe we can add an explicit scope to the test case in order to harmonize the result for master and older versions and DMD.

@s-ludwig
Copy link
Member

I can't really reproduce this within the ddox test suite: #240

The other question is why would a POD parameter be inferred as scope anyway? I thought scope should only have an effect on references?

@dkorpel
Copy link
Contributor Author

dkorpel commented Apr 18, 2023

It's probably caused by dlang/dmd#14561, which doesn't strip scope from the signature anymore because it causes forward reference errors.

@s-ludwig
Copy link
Member

Could this be fixed in the inference part, so instead of stripping the type in general, simply avoid tagging with scope_ during inference? I mean we could just adjust the test here, but that would mean that a lot of useless scope annotations would occur in real world documentation (plus lead to noise in call stacks and other places, of course).

@dkorpel
Copy link
Contributor Author

dkorpel commented Apr 18, 2023

I thought it wouldn't affect inferred scope, only explicit scope

@dkorpel
Copy link
Contributor Author

dkorpel commented Apr 18, 2023

How does ddox get this signature information from dmd? Is it using the json output?

@s-ludwig
Copy link
Member

Yeah, by default it uses the JSON output, preferring parsing mangled types and falling back to the "pretty" types and signatures. There is a also libdparse based alternative backend and we should probably also implement something based on dmd's parser, so that this would eventually become a non-issue. This will, however, require a little bit more work, because the live-generated documentation directly sources from the JSON file and wouldn't have access to the original sources (e.g. the multiple versions served by http://vibed.org/api/).

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 a pull request may close this issue.

3 participants