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

Rename fileVersion to dottedQuadFileVersion and specify format constraint #274

Closed
michaelcfanning opened this issue Nov 5, 2018 · 10 comments
Labels

Comments

@michaelcfanning
Copy link
Contributor

See tool.fileVersion. Expressed on invocation.tool.fileVersion and conversion.tool.fileVersion

      "pattern": "[0-9]+(\\.[0-9]+){3}"
@ghost ghost added bug non-substantive Change falls under editorial discretion labels Nov 6, 2018
@michaelcfanning michaelcfanning added the 2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. label Nov 6, 2018
@ghost
Copy link

ghost commented Nov 6, 2018

The schema says this, the spec doesn't have the words.

@ghost ghost self-assigned this Nov 7, 2018
@ghost
Copy link

ghost commented Nov 7, 2018

@michaelcfanning I think this issue is backwards. I don't think we should add words to the spec; I think we should remove the pattern from the schema.

The spec says:

If the operating system on which the tool runs provides a value for the file version of the tool's primary executable file, then the tool object MAY contain a property named fileVersion whose value is a string representation of that file version.

Sure, we know what Win32 and .NET represent the version with four dot-separated integers. But why should we require that? Maybe there's an OS that doesn't use that format. We talked about a representation that allowed any number of dot-separated numeric components, but why should we insist even on that? Just let it be a string, which is what the spec says today.

@michaelcfanning
Copy link
Contributor Author

The operative qualifier in your comment is 'maybe'. This property today is precisely constructed (in the schema validation) to produce a file version that's completely compatible with the Windows platform. i.e., the value is guaranteed to be a valid Windows/.NET file version. That's nice. If we loosen requirements for a 'maybe' value, we've lost the guardrails for the driving scenario that led us to define this property.

My suggestion is to preserve the existing schema and make it clear this is for the Windows platform which implements file version metadata. we could name the property windowsFileVersion if that helps.

I found some information on VMS file versioning. VMS is a versioned file system, which means that files don't have a version, per se, the operating system retains a history of files. You can basically go request an historical revision of a file. Versions increment from 1 to an upper bound. But! there are also some special values. '*" for example, means all versions. A negative number can be used to specify 'give me the nth version of a file before the most recent version'. These special designations are not likely to be useful in a SARIF context, however, where a producer would want to explicitly identify a precise file revision (since a relative revision only makes sense at the point in time and could be hard or impossible to compute later).

https://winscp.net/forum/viewtopic.php?t=14962

@ghost
Copy link

ghost commented Nov 8, 2018

I like your suggestion of renaming the property to windowsFileVersion -- it makes us completely transparent about the fact that we're adding something for the benefit of one particular OS.

Is there a trademark issue? (Like how you can't call your product "Windows File Munger", you have to call it "File Munger for Windows (TM)".)

@michaelcfanning
Copy link
Contributor Author

This is a property name within a larger format, we're not actually branding anything ourselves, or referring to a specific product, If you looks at MS general feedback on trademarks, I think it's clear that we're not in this general domain.

However, just to be sure perhaps we name this thing 'windowsPlatformFileVersion' which makes it quite clear we are providing adjectival specifiers that qualify the kind of file version, rather than embedding a reference to a specific Microsoft product. Presumably, we have established a pattern for other version properties if they materialize, vmsPlatformFileVersion, etc.

https://www.microsoft.com/en-us/legal/intellectualproperty/trademarks/usage/general.aspx

@ghost ghost added discussion-ongoing and removed to-be-written labels Nov 8, 2018
@michaelcfanning
Copy link
Contributor Author

I am fine with something like quadDottedFileVersion, as proposed in email.

@ghost
Copy link

ghost commented Nov 15, 2018

Ok, I'll make that change and add the schema constraint.

@ghost ghost added to-be-written design-approved The TC approved the design and I can write the change draft impact-breaks-consumers impact-breaks-producers and removed discussion-ongoing labels Nov 15, 2018
@ghost ghost changed the title Spec does not mention fileVersion population constraint captured in schema Rename fileVersion to dottedQuadFileVersion and add schema constraint Nov 15, 2018
@ghost ghost removed the non-substantive Change falls under editorial discretion label Nov 15, 2018
@ghost ghost changed the title Rename fileVersion to dottedQuadFileVersion and add schema constraint Rename fileVersion to dottedQuadFileVersion and specify format constraint Nov 18, 2018
ghost pushed a commit that referenced this issue Nov 18, 2018
@ghost
Copy link

ghost commented Nov 18, 2018

This issue is labeled both bug, because it adds the missing syntactic constraint, and design-improvement, because of the (presumably) improved property name.

@michaelcfanning
Copy link
Contributor Author

Presumed improved in the sense of allowing us to better conform to ISO standards around trademark use. :)

btw -based on the ISO standard, I suggest we go all in with their suggestion and update your text associated with EXAMPLE:

'On the Microsoft Windows (R) platform, this information is available in the FILEVERSION...'

Probably worth scrubbing for other 5 instances of 'Windows' use.

@ghost
Copy link

ghost commented Nov 23, 2018

Done. I also confirmed that Linux ® and Unix ® are registered trademarks, so I scrubbed for that too. I applied all the fixes directly in the provisional draft.

I filed #290, "Add registered trademarks as appropriate" so I'd have an issue number to mention in the Editor's Report.

ghost pushed a commit that referenced this issue Nov 29, 2018
ghost pushed a commit that referenced this issue Nov 29, 2018
@ghost ghost closed this as completed Nov 29, 2018
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant