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

doc (ABI comparison) and various other fixes #287

Closed
wants to merge 8 commits into from

Conversation

jnpkrn
Copy link
Contributor

@jnpkrn jnpkrn commented Jan 4, 2018

No description provided.

@jnpkrn jnpkrn changed the title doc fixes doc and various other fixes Jan 4, 2018
@jnpkrn jnpkrn force-pushed the fix-doc branch 2 times, most recently from e552a73 to 894a49a Compare January 4, 2018 20:58
@jnpkrn jnpkrn changed the title doc and various other fixes [WIP] doc and various other fixes Jan 4, 2018
@jnpkrn
Copy link
Contributor Author

jnpkrn commented Jan 4, 2018

Actually, I was wrong about the cause, which is likely rather the same as in
lvc/abi-compliance-checker#64

Todo:

Contingency strategy: downgrade to sub-fedora-26 abi-compliance-checker
and try again.

@jnpkrn
Copy link
Contributor Author

jnpkrn commented Jan 5, 2018

Hmm, see also lvc/abi-compliance-checker#69

(It made a service as-was, but being afforded more time, this would
have accompanied that commit right away, for better understanding,
brevity and uniformity.)

Signed-off-by: Jan Pokorný <[email protected]>
There was a significant redundancy wrt. build flags and EXTRA_DIST
assignment (the latter become redundant as of f6e4042 at latest)
spread all over the place (vivat copy&paste).  Also, in one instance,
CPPFLAGS (used) was confused with CFLAGS (meant).

Signed-off-by: Jan Pokorný <[email protected]>
@jnpkrn
Copy link
Contributor Author

jnpkrn commented Jan 10, 2018

./check abi now uses abi-dumper for creating "ABI dumps" as
abi-compliance-checker won't work out-of-the-box in the current shape
(see lvc/abi-compliance-checker#70), and appears
to be preferred choice for this purpose, anyway.

As an aside, we should consider integrating with abidiff, though it seems
to lack direct HTML output:
https://fedoraproject.org/wiki/How_to_check_for_ABI_changes_with_abidiff

@jnpkrn jnpkrn changed the title [WIP] doc and various other fixes doc (ABI comparison) and various other fixes Jan 10, 2018
1. ABICC >= 2 needs to be passed -cxx-incompatible switch because C is
   no longer a default for this tool (used to be vice versa),
   plus current version will stop choking on C vs. C++ (our C code with
   C++ compatibility wrapping being viewed from C++ perspective for the
   purpose of dumping the declared symbols, which somewhat conflicts
   with internal masking of the C++ keywords being used as valid C
   identifiers [yet some instances must not be masked here, see
   lvc/abi-compliance-checker#64) only
  if _also_ something like this is applied:
   lvc/abi-compliance-checker#70

2. since 20246f5, libqb.so no longer poses a symlink to the actual
   version-qualified shared library, but rather a standalone linker
   script, which confuses ABICC, so blacklist that file for the scanning
   purposes explicitly, together with referring to the library through
   it's basic version qualification (which alone, sadly, is not
   sufficient as ABICC proceeds to scan whole containing directory
   despite particular file is specified)

Signed-off-by: Jan Pokorný <[email protected]>
Beside avoiding issues with abi-compliance-checker in the role of ABI
dumps producer (see the preceding commit), it also seems to generate
more accurate picture (maybe because it expressly requires compiling
with debugging information requested).

Signed-off-by: Jan Pokorný <[email protected]>
Primary trigger was the fact that "./check abi" (via
build-aux/generate-docs) choked on that because abi-compliance-checker
(ABICC) uses g++ to list symbols in header files playing hide-and-see
regarding C identifiers otherwise conflicting with C++ keywords,
which is moreover triggered only with -cxx-incompatible switch
since ABICC 2.0 (and even then the respective code needs some further
tweaking: lvc/abi-compliance-checker#70),
and this switch wasn't historically applied.

Anyway, this C++ perspective there was likely unprecedented even
though libqb no doubt targets such compatiblity.  To avoid its
breakage within the public headers' scope in the future, arrange
for checking it regularly alongside the "auto_check_header" tests
(cf. make -C tests check-headers).

Signed-off-by: Jan Pokorný <[email protected]>
IIRC, Chrissie asked about this around inclusion of the test at
hand, and it seemed there was no way but to emit a warning to get
something output at all.  Now it turns wrong, and moreover, we
make the code not fixed on GCC specific pragmas, with a bit of
luck, "#pragma message" approach is adopted more widely by compilers.

Signed-off-by: Jan Pokorný <[email protected]>
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016 Red Hat, Inc.
* Copyright 2018 Red Hat, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

A digression about copyrights:

The goal of a copyright notice is to show priority, not currency, so the initial year of creation should always be listed. If there is ever a dispute over who created the work, it establishes a claim for when it was created (earliest wins). It can also imply the date the work enters the public domain (which varies by country, but is a certain amount of time after the work's creation or the author's lifetime).

There is no universal form for how copyright notices on frequently-updated works should be written, but my personal preference is a year range, e.g. "Copyright 2016-2018 ...".

The "(c)" is indeed irrelevant from the perspective of copyright law and can be dropped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the age of git, I suppose solely newest date makes the most sense,
it provides a basic estimate and if anyone needs exact legal dispute,
git provides exact data easily.

Copy link
Contributor Author

@jnpkrn jnpkrn Jan 22, 2018

Choose a reason for hiding this comment

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

I should have said "worst case estimate", which makes whole a lot of sense,
at least to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

In the age of git, I suppose solely newest date makes the most sense,
it provides a basic estimate and if anyone needs exact legal dispute,
git provides exact data easily.

Certainly, the git history provides a good claim to priority, but the copyright notice here is not valid.

See the U.S. Copyright Office's "Compendium of U.S. Copyright Office Practices" [ https://www.copyright.gov/comp3/ ], particularly "Chapter 2200: Notice of Copyright", sections 2205.1(A) and 2205.1(F), or for a much more readable summary:

https://techwhirl.com/updating-copyright-notices/

The original date of publication must be in the notice for it to be valid. Giving a year range indicates that portions of the work were added in different years, though it cannot say which portions are which -- that is where the git history comes in handy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the footer on redhat.com main page, then (sigh)

Copy link
Contributor Author

@jnpkrn jnpkrn Jan 23, 2018

Choose a reason for hiding this comment

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

Looking deeper in your linked article:

What happens when the manual is revised? Interestingly, the
Copyright Act doesn’t directly say. But the commonly accepted
practice is to include multiple years in the copyright notice,
indicating the various years in which various material in the
overall work was first published.

It may make sense for the material without rigorously tracked
history, but again, it's not the case with VCS-tracked project
(unless it's history got unclear on the initial import, which
is, again, not the case with libqb).

Furthermore, per
https://www.copyright.gov/help/faq/faq-definitions.html#notice:

While use of a copyright notice was once required as a condition
of copyright protection, it is now optional.

So I see no contradiction when using mere single date (newest), making
it an estimate (version control will always have the last say) when
the given file at the current form goes out of copyright protection
(country-dependent as mentioned) and putting negligible (because of
VCS) onus on figuring out the exact data about its parts to anyone
wanting to use them beyond copyright-granted protection packed with
LGPL-granted provisions (that should be enough in most cases anyway).
Fair deal, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Purely as a matter of style I prefer the date "from-to" format as it gives some idea how long the file has been there without having to dig into the VCS history. Also, when things have been moved from one repo to another the dates can get unreliable. I'm not saying that's happened here, but if you keep the dates inside the file then you can be sure that it doesn't get lost ... unless someone removes it ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

The legal form of the copyright notice is intentionally ambiguous to allow variety, but one point it is always clear on is that the original date of publication must be listed. The initial part may be "Copyright", "Copr.", or the copyright symbol, and the year may be in just about any recognizable form (numerals, spelled out, Roman numerals, etc.), but it must specify the year of initial publication. The git history is good for determining which portions of the work were created in which years, but the copyright notice should list the original year, the original year plus later years of modification, or a range from the original year to the year of latest modification.

@chrissie-c
Copy link
Contributor

Is this patch ready for commit? If so, please 'fix' the copyright date and I'll pull it. If not is there anything more that needs doing?

@jnpkrn
Copy link
Contributor Author

jnpkrn commented Jan 30, 2018

The only fix I am in favour of is to keep the original date if you think
the change is trivial enough. Tracking the ranges just establishes
false sense of "definite answer around originality/priority of the code",
which is likely bogus in some cases already as the code was transplanted
from original corosync (openais?) codebase (e.g. logging thread for sure).

Otherwise yes, the PR is ready for merging if ./check abi works for
you now.

@chrissie-c
Copy link
Contributor

updated as #324

@chrissie-c
Copy link
Contributor

Merged as #324

@chrissie-c chrissie-c closed this Sep 25, 2018
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.

3 participants