json-ld / yaml-ld

CG specification for YAML-LD and UCR
https://json-ld.github.io/yaml-ld/spec
Other
19 stars 8 forks source link

Remove `%YAML 1.*` directives #125

Closed anatoly-scherbakov closed 8 months ago

anatoly-scherbakov commented 8 months ago

Preview | Diff

ioggstream commented 8 months ago

@gkellogg I'd leave the Yaml headers in the tests to ensure parsers don't use Yaml 1.x by default

gkellogg commented 8 months ago

@ioggstream you might look at the Oct 18 minutes where this was discussed. It was felt that the specific version of YAML used was somewhat orthogonal to the use within YAML-LD, other than for having a normative reference and to leverage the media-type, of course.

A practical issue (for me, anyway) is that the version of libYaml included in GH Actions on Linux doesn't seem to support the %YAML 1.2 directive (see run log here, for example). Generally,, the differences between YAML 1.2 and earlier versions (yes/true etc) are orthogonal to the YAML-LD spec, itself, and while they should be noted (informally), it shouldn't be a barrier for implementations.

As it happens, installing libyaml-daev on Linux gives "libyaml-dev is already the newest version (0.2.2-1build2)". I haven't researched further to see if there's a newer library that can be added (you'd think -dev would be enough). The same code runs on Mac okay.

TallTed commented 8 months ago

[@ioggstream] @gkellogg I'd leave the Yaml headers in the tests to ensure parsers don't use Yaml 1.x by default

Maybe there should be additional tests, with notes about their expected results, both pass and fail (as a failed test doesn't necessarily indicate a problem...), which may depend on the features/functionality of the test environment's libraries, e.g., the GitHub CI environment has an older C library for YAML, which doesn't handle the 1.2 declarations, while @gkellogg's local environment has a newer C library, which handles the 1.2 declarations just fine....

ioggstream commented 8 months ago

the version of libYaml included in GH Actions on Linux doesn't seem to support the %YAML 1.2 directive

While some YAML libs may opt for a constrained yaml version (for ease/performance/whatever), I won't use them for testing the interoperability of the specifications. OTOH it is possible to "preprocess" well-formed files before passing them to implementation-interoperability tests , when the implementations are known not to be YAML compliant.

IMHO an implementation that does not support %YAML is problematic, since

A version 1.2 YAML processor must accept documents with an explicit “%YAML 1.2” directive, as well as documents lacking a “YAML” directive.

anatoly-scherbakov commented 8 months ago

The tests as they are now (without the headers) are actually indicative of the YAML compatibility issues. I mean, if an implementation doesn't by default expect a document to be YAML 1.2 then the implementation is non-compliant.