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

Errata and boilerplate #7

Closed
wants to merge 11 commits into from
Closed

Errata and boilerplate #7

wants to merge 11 commits into from

Conversation

gkellogg
Copy link
Member

@gkellogg gkellogg commented Jan 27, 2023

This addresses #4, other than the unresolved issues about IRIs.


💥 Error: 500 Internal Server Error 💥

PR Preview failed to build. (Last tried on Jan 30, 2023, 10:55 PM UTC).

More

PR Preview relies on a number of web services to run. There seems to be an issue with the following one:

🚨 Spec Generator - Spec Generator is the web service used to build specs that rely on ReSpec.

🔗 Related URL


😭  Sorry, there was an error generating the HTML. Please report this issue!
Specification: http://labs.w3.org/spec-generator/uploads/WerS0g/spec/index.html?isPreview=true%3FisPreview%3Dtrue&publishDate=2023-01-30
ReSpec version: 32.6.1
File a bug: https://github.com/w3c/respec/
Error: Error: Evaluation failed: Timeout: document.respec.ready didn't resolve in 28248ms.
    at ExecutionContext._ExecutionContext_evaluate (/u/spec-generator/node_modules/puppeteer-core/lib/cjs/puppeteer/common/ExecutionContext.js:229:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ExecutionContext.evaluate (/u/spec-generator/node_modules/puppeteer-core/lib/cjs/puppeteer/common/ExecutionContext.js:107:16)
    at async generateHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:221:12)
    at async toHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:92:18)
    at async Object.generate [as respec] (file:///u/spec-generator/generators/respec.js:15:44)
    at async file:///u/spec-generator/server.js:252:48

If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.

@gkellogg gkellogg requested review from afs and pchampin January 27, 2023 23:41
@gkellogg
Copy link
Member Author

Still seeing ReSpec errors even after commenting out the section that seemed to be the problem. Can't reproduce on my own system. This is also apparently preventing PR preview from running. Could the addition of submodules play a role?

@gkellogg
Copy link
Member Author

Note, the timeout is from https://labs.w3.org/spec-generator/, when using https://raw.githubusercontent.com/w3c/rdf-concepts/errata-and-boilerplate/spec/index.html. Says to file a bug, which it may come to.

Sorry, there was an error generating the HTML.
Please report this issue!
Specification: http://labs.w3.org/spec-generator/uploads/6dGzrm/spec/index.html?publishDate=2023-01-27
ReSpec version: 32.6.1
File a bug: https://github.com/w3c/respec/
Error: Error: Evaluation failed: Timeout: document.respec.ready didn't resolve in 28671ms.
    at ExecutionContext._ExecutionContext_evaluate (/u/spec-generator/node_modules/puppeteer-core/lib/cjs/puppeteer/common/ExecutionContext.js:229:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async ExecutionContext.evaluate (/u/spec-generator/node_modules/puppeteer-core/lib/cjs/puppeteer/common/ExecutionContext.js:107:16)
    at async generateHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:221:12)
    at async toHTML (/u/spec-generator/node_modules/respec/tools/respecDocWriter.js:92:18)
    at async Object.generate [as respec] (file:///u/spec-generator/generators/respec.js:15:44)
    at async file:///u/spec-generator/server.js:252:48

Copy link
Contributor

@afs afs left a comment

Choose a reason for hiding this comment

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

One comment - not blocking.

Process-wise this PR looks good.

spec/index.html Outdated Show resolved Hide resolved
@gkellogg
Copy link
Member Author

I’ve been trying to track down why the GH Actions build fails, and why PR Preview also fails. It seems the the problem started when we introduced the git submodule for /spec/common. I note that the https://w3c.github.com/rdf-concepts/spec link renders okay, but not through either GitHack or W3c spec-generator using the raw file view.

The pre-submodule version renders fine, the “main” version errors with a timeout. I just marked the last commit prior to using the submodule as the branch "pre-submodule" just for this purpose.

PR Preview uses spec-generator, so that’s why it fails. I’m not sure why respec fails in the GH Action, but it must be due to the same problem. While there might be something we could do in the GH Action config to address this, there’s really nothing we can do about PR Preview and spec-generator.

I wasn’t able to see anything posted about this through a Google search, so it might not be a completely understood problem, but the fact that GitHack fails the same way suggests there’s probably nothing that can really be done. We may be faced with the prospect of undoing the recent change to use Git submodules, unfortunately.

@Tpt
Copy link
Contributor

Tpt commented Jan 29, 2023

@gkellogg: This might help: #8

@gkellogg gkellogg force-pushed the errata-and-boilerplate branch from baec7f9 to 578d6ff Compare January 29, 2023 18:29
@gkellogg
Copy link
Member Author

Thanks @Tpt, that helped the GH Action get further, but PR Preview seems to continue to fail. Notably, spec-generator (used by PR Preview) still fails, as it doesn't have visibility to the submodule, apparently. Running spec-generator using the pre-submodule branch does work.

Copy link
Contributor

@pchampin pchampin left a comment

Choose a reason for hiding this comment

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

Appart from losing the previous contributors, LGTM

spec/index.html Outdated
<section class="appendix informative" id="changes">
<h2>Changes between RDF 1.0 and RDF 1.1</h2>
<p>The following people have contributed to this specification:
<span id="gh-contributors"></span>
Copy link
Contributor

Choose a reason for hiding this comment

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

A number of the previously listed people will not be listed anymore... I think that's a shame...

Copy link
Contributor

Choose a reason for hiding this comment

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

Apart from that, that's a super cool feature of Respec hat I didn't know about ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

We certainly need to figure out how to deal with 1.1 contributors, in addition to 1.0. A reference to the contributors section in the previous spec(s) would be one way, or multiple contributor sections. Ultimately, this could prove to be difficult to manage. But, I think it’s a group decision on how to recognize previous contributions. Note that we do maintain a list of Former Editors separately.

I think @iherman had a script to get the current WG membership and create a list for inclusion automatically.

similarly, what level of the “previous changes” do we try to maintain? Some have a lot of transition detail (WD, CR, PR, REC) that isn’t as pertinent now.

Copy link
Member

Choose a reason for hiding this comment

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

@gkellogg I think what you referred to is here:

https://github.com/w3c/publ_ack

I have not looked at the script for a while, because recent groups that I worked with had their own way of creating the list of acknowledgements, but I presume it is still operational...

Copy link
Member Author

Choose a reason for hiding this comment

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

Just came up with this using the W3C API: w3c/rdf-common#3. (With an assist from ChatGPT).

Copy link
Member

Choose a reason for hiding this comment

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

Yep, that is what my script uses as well!

… in an ednote about retrieving WG participants from group page.
…tionalization Considerations" sections. Security Considerations taken from N-Triples, but made more generic.
Comment on lines +1285 to +1286
Application rendering strings retrieved from untrusted RDF documents must ensure
that malignant strings may not be used to mislead the reader.
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe this requirement can be satisfied by any applications available today, if ever.

As things stand, humans must take care to load varyingly trustworthy RDF into appropriately partitioned data stores (typically, using named graphs), and to execute queries across "all data in the store" only when recognizing that results may include malicious, erroneous, or otherwise undesirable statements (whether triples or quads).

Such undesirable results are not the responsibility of any software in the stack.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's taken right out of the Turtle IANA Considerations section

Turtle can express data which is presented to the user, for example, RDF Schema labels. Application rendering strings retrieved from untrusted Turtle documents must ensure that malignant strings may not be used to mislead the reader. The security considerations in the media type registration for XML ([RFC3023] section 10) provide additional guidance around the expression of arbitrary data and markup.

I didn't want to try to tread new ground here, but ReSpec demands that specs have such a section, and this seemed to apply generally to RDF serializations. I also don't want to revisit the IANA registrations of Turtle/TriG/N-Triples/N-Quads which already have such considerations, and it would be inconsistent to say something different.

Copy link
Member

Choose a reason for hiding this comment

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

Wow. As I understand things, that means that zero current implementations of Turtle/TriG/N-Triples/N-Quads functionality are spec compliant, because none can "ensure that malignant strings may not be used to mislead the reader" — there's just no way to do so!

Copy link
Contributor

Choose a reason for hiding this comment

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

The text should say "Applications" to be clearer as it does a little further up.

The text is in a section titled "Security considerations" - things applications need to consider. It's not compliance.

Parsers aren't applications and don't render strings.
Applications choose to use parsers and data and the applications are responsible for the usage and so responsible for misleading the user.

The text is an obligation and noteworthy point on the use of the data.

Copy link
Member

Choose a reason for hiding this comment

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

Followups to this thread should go on #11.

spec/index.html Outdated Show resolved Hide resolved
spec/index.html Outdated Show resolved Hide resolved

<section id="internationalization">
<h2>Internationalization Considerations</h2>
<p>RDF is restricted to representing string values with -to-right or right-to-left direction indicators.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<p>RDF is restricted to representing string values with -to-right or right-to-left direction indicators.
<p>RDF is restricted to representing string values with left-to-right or right-to-left direction indicators.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
@gkellogg
Copy link
Member Author

Closing in favor of #10, which incorporates suggestions. Discussions of Security Considerations may need to be a new issue potentially resulting in another PR, but it is a meta-issue affecting at least four other specs.

@gkellogg gkellogg closed this Jan 31, 2023
@gkellogg gkellogg deleted the errata-and-boilerplate branch February 1, 2023 23:07
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.

6 participants