checksrc: repair the copyrightyear check

[bagder]

I'm not sure why or how, but checksrc's copyrightyear check has stopped working and it seems to be because current git doesn't show the commit date for the commands used. #4547 made me realize.

This patch brings back the warnings for me. Please give it a look @danielgustafsson!

[bagder]

The problem showed up with git version 2.24.0.rc1 (and rc2) on Debian Linux on two of my machines.

[danielgustafsson]

LGTM, thanks

[bagder]

I haven't investigated exactly why, but when fixing this problem and reran make checksrc in the source root (with this warning enabled in a few source directories), it pointed out a large number of copyright range warnings. Several of them being of the kind where the range ends with year X (not 2019) but the file was last modified in year Y.

I will fix these by updating their ranges to - 2019 since we can't backdate the fixes and once I change these files, they will be updated in 2019!

I also think we need to put back the copyright year check in the CI to catch these earlier.

[danielgustafsson]

I haven't investigated exactly why, but when fixing this problem and reran make checksrc in the source root (with this warning enabled in a few source directories), it pointed out a large number of copyright range warnings. Several of them being of the kind where the range ends with year X (not 2019) but the file was last modified in year Y.

IIRC (with a disclaimer on the c part), the COPYRIGHTYEAR check only runs when explicitly invoked or when in maintainer mode. It seems quite likely that these snuck through on patches which were developed outside of maintainer mode, perhaps trivial PRs which didn't end up being massaged in a maintainer tree where it would've been caught.

I will fix these by updating their ranges to - 2019 since we can't backdate the fixes and once I change these files, they will be updated in 2019!

I guess we could add logic to COPYRIGHTYEAR to cope with fixup commits like this one, but I doubt it's worth the effort.

I also think we need to put back the copyright year check in the CI to catch these earlier.

It was removed as it was "yelling at contributors" too much, causing red builds everywhere. We could shift the responsibility to the committer and not the contributor but it won't fix the red build. Not sure how to best tackle that.

[bagder]

It was removed as it was "yelling at contributors" too much, causing red builds everywhere

Yeah, and that's probably still the case. I should probably be fine with having checksrc fixed since then at least I will get these errors correctly in my local build - and then not add it to the CI builds for that reason.

[bagder]

Now this is really curious. This travis build warnings say the commit year is 2019 while it clearly is 2018 for several files (for example curl_range.c and arpa_telnet.h) and when I debug the checksrc on my host it says 2018... ?

[danielgustafsson]

Now this is really curious. This travis build warnings say the commit year is 2019 while it clearly is 2018 for several files (for example curl_range.c and arpa_telnet.h) and when I debug the checksrc on my host it says 2018... ?

That's borderline bizarre. Since I assume that Travis isn't screwing around in the .git index, the only suggestion would be that the files somehow was perceived as edited by Git (the check falls back on the current year for any file with modifications). Perhaps a printf debug line which identifies which checksrc check decides on the year?

[mback2k]

Are you working on a fresh checkout/clone? Git normally creates files with the current date on checkout and does not care about filesystem timestamps at all.

[danielgustafsson]

On 5 Nov 2019, at 12:39, Marc Hörsken ***@***.***> wrote: Are you working on a fresh checkout/clone? Git normally creates files with the current date on checkout and does not care about filesystem timestamps at all.

The check inspects the commit timestamp, not the file timestamp.

[jay]

hm
I get

$ git rev-list --max-count=1 --timestamp HEAD -- ./arpa_telnet.h
1539867109 ad547fcf7b3c0191f63396b94c797bfbb4147f62

yet the checksrc debug output shows:

y: ./arpa_telnet.h 2019 (1570699101 622cf7db6bd2d943e67660020b3984e6068f1084)
./arpa_telnet.h:10:25: error: Copyright year out of date, should be 2019, is 2018 (COPYRIGHTYEAR)
  * Copyright (C) 1998 - 2018, Daniel Stenberg, <daniel@haxx.se>, et al.

edit: super weird how all the bad ones seem to be 1570699101 622cf7db6bd2d943e67660020b3984e6068f1084)

[jay]

I added a commit and the CI results for it show the same weird nonsense BUT this time with 0dc14b83818f6166d60e939ce2cc1c4806d39c4a, which interestingly is the child of 622cf7db6bd2d943e67660020b3984e6068f1084 (the prior nonsense commit). I still have no idea what's going on.

[jay]

I ran this command which should show the file history in reverse chronological order but instead it just shows one entry:

`git rev-list --max-count=99 --timestamp HEAD -- ./arpa_telnet.h 1>&2`

1570913113 e80b5c801652bdd8aa302345954c3ef8050d039a

12063.1:
y: ./arpa_telnet.h 2019 (1570699101 622cf7db6bd2d943e67660020b3984e6068f1084)
then 12070.1:
y: ./arpa_telnet.h 2019 (1570742415 0dc14b83818f6166d60e939ce2cc1c4806d39c4a)
then 12072.1:
y: ./arpa_telnet.h 2019 (1570911046 02e608f0bb42a7749a3737eea8ab28f87dcd8f46)
then 12076.1:
y: ./arpa_telnet.h 2019 (1570913113 e80b5c801652bdd8aa302345954c3ef8050d039a)

none of this makes any sense. each one of those commits is a direct descendant of the prior one and have nothing to do with that file.

[jay]

I think I understand what's happening. Travis uses a truncated history on clone:

$ git clone --depth=50 https://github.com/curl/curl.git curl/curl

hm:

$ git rev-list --count e80b5c801652bdd8aa302345954c3ef8050d039a..bagder/checksrc-copyrightyear
51

repro:

$ git clone --depth=50 https://github.com/curl/curl.git
Cloning into 'curl'...
remote: Enumerating objects: 3654, done.
remote: Counting objects: 100% (3654/3654), done.
remote: Compressing objects: 100% (2989/2989), done.
remote: Total 3654 (delta 1342), reused 1340 (delta 636), pack-reused 0
Receiving objects: 100% (3654/3654), 3.87 MiB | 3.98 MiB/s, done.
Resolving deltas: 100% (1342/1342), done.
Checking connectivity... done.

$ cd lib

$ git rev-list --max-count=99 --timestamp HEAD -- ./arpa_telnet.h
1570913113 e80b5c801652bdd8aa302345954c3ef8050d039a

It appears if there are files that have not been modified in the truncated history then git returns the first commit it has. git rev-list has a --min-parents= option and if I set it to 1 then I don't get any results back. So that's a workaround unless you guys want to clone the complete history, but given that it's run all the time I don't know that it's necessary to do that as long as the initial copyright years for the old files were fixed.

I put in a commit that does it let's see what happens...

[bagder]

Aha! How silly that it took that long to figure out! Good catch there @jay!

1. I'm not sure we need to work on this much more since I don't think we want this check done in travis anyway and it works locally if we just don't limit the clone depth
2. Apparently it is possible to tell travis to not set the limit to 50: git: depth: false

[jay]

• I'm not sure we need to work on this much more since I don't think we want this check done in travis anyway and it works locally if we just don't limit the clone depth
• Apparently it is possible to tell travis to not set the limit to 50: git: depth: false

--min-parents=1 worked in Travis. As long as the change is in the last 50 commits, and since Travis runs every commit, and since you fixed all the current problems, it should catch any future problems. So I think increasing the depth is unnecessary.

I've squashed your commits into 2, if it passes we should be done.

[jay]

Also for future reference if that didn't work my next step would've been to ssh into travis during the build (looks neat).

[bagder]

Right, this fix should make the build on travis happy. But I'm still not convinced we should add this copyright check to the travis build for the same reasons as before: it causes a great deal of (surprise) complaints on PRs for users. My intent was just to make sure that it works on travis and then not push it but only run these checks in my local builds.

[jay]

Ok np I'll leave it to you to wrap up.

[bagder]

I will let the copyrightyear scan be enabled in include/curl and docs/examples for now to see if that's a good idea ...