Use p4 annotate's -I argument instead of -i

[mjdetullio]

See #7

[tyrel]

So..... guess what? Another team at my company is now having issues with this. It turns out for THEM, using capital -I shows a changelist number on the remote Perforce server, while -i does not -- the opposite of the files on my depot. It must have something to do with exactly in what manner those files were merged in from the other server....

[henryju]

Do you have a way to get in touch with Perforce support so that we could find a solution to this (or at least understand the behavior)? I'm sorry we (SonarSource) have a very limited knowledge of Perforce. Only by reading the documentation I'm not sure to understand the difference between -i and -I.

[tyrel]

I'm doing some investigation to try to determine the best solution. (Our long-term solution is to move to Git, but... yeah.) I'll let you know what I can come up with.

On Mon, Jan 18, 2016 at 1:02 PM Julien HENRY notifications@github.com wrote:

Do you have a way to get in touch with Perforce support so that we could find a solution to this (or at least understand the behavior)? I'm sorry we (SonarSource) have a very limited knowledge of Perforce. Only by reading the documentation I'm not sure to understand the difference between -i and -I.

— Reply to this email directly or view it on GitHub https://github.com/SonarSource/sonar-scm-perforce/pull/9#issuecomment-172653049 .

[tyrel]

First of all, IntelliJ IDEA has updated their Perforce plugin as of 15.0.2 to no longer use -I, ever. It always uses -i if it's supported by the server, otherwise neither.

That means as of 15.0.2, when I look at the annotate inside of IDEA, I'm seeing the "bad" changelist. However, it doesn't blow up: it shows these as "remote" and includes the change date.

I enabled logging in the IntelliJ IDEA Perforce Integration plugin to see what commands it's running in order to find the information it needs: It does filelog -i -l -t -m 1000 [path], then it does annotate -q -i -dw [path].

I was trying to find, but couldn't, what exactly what the issue was that filelog wasn't sufficient for you before?

I'd be willing to spend a little bit of time trying to figure out a good solution to everything, if I can understand all the problems, and submit a pull request.

[mjdetullio]

I don't know the exact circumstances that reproduce the problem, but this is why I switched p4 filelog to p4 change:

Given that a file produced p4 annotate results such as

10021:package com.example;
10021:
10021:public class MyClass {
10021:
10022:    public static boolean invert(boolean value) {
10023:        return !value;
10022:    }
10024:
10025:    public static int add(int a, int b) {
10025:        return a + b;
10024:    }
10021:
10021:}

When you run p4 filelog, the expectation is that it contains all the changelists from the annotation.

In this example, there are 5 changelists in the annotation, 10021 thru 10025. In some instances, p4 filelog would not provide a history that contained all these changelists. For example, it would contain 10021, 10022, 10024, and 10025 -- missing 10023. Similarly, adding -h to the command would return 10021, 10022, 10023, and 10024 -- missing 10025.

This resulted in exceptions (in the Compute Engine with SQ 5.2 or during analysis with SQ 5.3) because null was assigned for the date and author on the lines annotated with the changelist missing from the p4 filelog results.

Running p4 filelog twice was far too complex when it came down to implementation and it turns out p4 change was more performant in some instances.

Technically, either method is supposed to be sufficient, regardless of whether its p4 filelog or p4 change -o on each changelist. For my situation in this example, it just so happened that using p4 change -o on each changelist was more reliable.

[mjdetullio]

And actually in Perforce 2015.2, the -u argument was introduced for the p4 annotate command that provides the user and date information, which removes the need for running p4 filelog or p4 change.

The problem with that is, 1- it's still very new (not even ${dayJob}'s servers are upgraded yet), so using that feature exclusively would alienate a lot of users and 2- we would need to request that Perforce publish the 2015.2 version of the p4java library to Maven Central.

[tyrel]

I'm working on a solution now. I'm going to do filelog then annotate, and if I run across a changelist in the annotate that wasn't in the filelog, I'll do a change to look it up. If all that fails, then set the date to epoch and the name to "unknown" (a graceful failure).

We're running it through functional and integration tests and if it works well, I'll submit a pull request later.

[tyrel]

By the way, the 2015.2 version of p4java is in Maven Central, as of 12 days ago. An option there would be to check the server's version and use annotate -u if it's new enough, otherwise fall back to this other behavior.

[tyrel]

Added PR #10

[henryju]

I quickly had a look at 2015.2 and it seems the -u option is not exposed in the API :( Is there any other advantage to update to 2015.2? Maybe to get some bug fixes?

[tyrel]

Indeed, the IFileAnnotation interface isn't changed at all. That's annoying.

Here are the bug fixes and enhancements noted in p4java's release notes for 2015.2:

Bugs fixed in 2015.2

#1308053 (Bug #83624)
    Fixed a potential problem where busy concurrent p4 login and
    p4 logout commands could wipe out (loose) tickets in the
    P4TICKETFILE file by creating a shadow P4TICKETFILE lock
    (".lck") file.

#1305655 (Bug #80437)
    Fixed an issue where large files (size > 2GB) are being
    truncated when synced on the Windows plaform.

#1305257 (Bug #80413)
    Fixed an issue when syncing files with size > 2GB (number of
    bytes > java.lang.Integer.MAX_VALUE).

#1244354 (Bug #81080)
    Fixed an issue with P4Java connecting and authenticating to
    a Perforce server with (man-in-middle) security configurable
    'net.mimcheck' set to value >= 4.

#1237901 (Bug #80389)
    Fixed a file corruption issue with P4Java shelving/unshelving
    of utf16-le files in the Windows environment.

#1244455 (Bug #70733)
    Fixed a potential data loss problem with P4Java's handling of
    protections. P4Java now uses the innate ordering of Java List
    instead of explicitly setting the order of each protection entries.

#1234279 (Bug #67229)
    Fixed an issue where the 'integrate' command fails if the current
    client set on P4Java is not "myclient" when executing myclient's
    integrateFiles() methhod.

#1222114 (Bug #62825)
    Fixed an issue where the 'shelve' command fails if the current
    client set on P4Java is not "myclient" when executing myclient's
    shelveChangelist() methhod.

Minor new functionality in 2015.2

#1310414 (Bug #83909)
    Add support for "SpecMap" in the depot spec. For spec depots,
    the optional description of which specs should be saved, as
    one or more patterns. For example, "-//spec/client/*_tmp*".

#1310411 (Bug #83908)
    Add support for "tangent" depot type. It defines a read-only
    location which holds tangents created by the 'fetch -t' command.
    The tangent depot named 'tangent' is automatically created by
    'fetch -t' if one does not already exist.

#1310141 (Bug #83907)
    Added support for the "StreamDepth" field in the depot spec.
    For stream depots, the optional depth to be used for stream
    paths in the depot, where depth equates to the number of
    slashes following the depot name of a stream. This field is
    referenced when streams are being created. The default is '1',
    matching the traditional stream name. This value may not be
    updated once streams or archive data exist within the depot.

#1243317 (Bug #80744)
    Added support for stream path type 'import+'. It is the same
    as 'import' except that files can be submitted to the import
    path. This feature requires Perforce server version >= 2014.2.

I don't think any of this will affect the usage by sonar-scm-perforce, so there's probably not advantage to upgrading at this time.

[henryju]

Do you have some way to contact Perforce support and ask them if they plan to add the "-u" flag to p4java?

[tyrel]

I can probably get the guy who manages our Perforce servers to get in touch with them about it. I'll send him an e-mail and find out.

[tyrel]

FYI, I sent Perforce a support request 21 days ago and never received any response... but they did add me to their mailing list so I get all their spam now!

[henryju]

@tyrel ^^