Added arm64 support to .travis.yml file

[odidev]

Added arm64 support to .travis.yml file.

1. JDK 11 is used to build jna on arm64. Thus, "openjdk-11-jdk" will be installed and JAVA_HOME and PATH will be set according to that.
2. libltdl-dev package needs to be installed for supporting arm64.

[rhenwood-arm]

This commit message should start with a one line summary that described the change. (I work with @odidev )

EDIT: I think I might have reviewed an old change? Either way, I think this PR could be cleaned up.

[matthiasblaesing]

The change itself looks sane to me. Indeed the change should be squashed into one commit with a summary line, an empty line and following that the details description. Thank you.

[dbwiddis]

I spent a few hours yesterday banging my head against this file and noted a few other changes that could be made as long as we're updating the file:

• the sudo: false line is deprecated/useless and can be removed
• I'm not sure the purpose of the || true when following the if ... fi blocks. Should it be there?
• Travis has a limit of 5 concurrent builds (2 on osx) for open source projects . This change adds a 6th build. We might consider removing another one (why do we do JDK12?)

[matthiasblaesing]

I spent a few hours yesterday banging my head against this file and noted a few other changes that could be made as long as we're updating the file:

* the `sudo: false` line is deprecated/useless and can be removed

Agreed.

* I'm not sure the purpose of the `|| true` when following the `if ... fi` blocks. Should it be there?

They are there to force a successful result state - else the test is marked as failed.

* Travis has a limit of 5 concurrent builds (2 on osx) for open source projects . This change adds a 6th build. We might consider removing another one (why do we do JDK12?)

What happens after 5? Are they run sequentially?

[dbwiddis]

These nested if statements could be simplified with an &&

[dbwiddis]

What happens after 5? Are they run sequentially?

Yes, that's the concurrent limit. Job # 6 (in this case the osx one) will sit waiting until at least one of the others has finished. (And the osx jobs also tend to be the ones that have to wait on resources.)

[dbwiddis]

You can see, for example, in the tests run for this PR, the first 5 jobs started at 0:56:00 to 0:56:03 but the osx job didn't start until 0:58:23, corresponding to when job 2 completed after 2:19.

[matthiasblaesing]

What happens after 5? Are they run sequentially?

Yes, that's the concurrent limit. Job # 6 (in this case the osx one) will sit waiting until at least one of the others has finished. (And the osx jobs also tend to be the ones that have to wait on resources.)

I could live with that. The JDK12 job however is not really needed. It maybe, that at time of creation some special problem with JDK 12 was reported/observered, but I'm more attached to the ea (newest) job.

[dbwiddis]

Agree re: JDK12. Wouldn't mind testing vs. the current release (JDK 14) plus EA, however.

[dbwiddis]

They are there to force a successful result state - else the test is marked as failed.

I understand this for the one-liner conditionals, they're absolutely necessary in this case:
- '[ "${TRAVIS_OS_NAME}" = "osx" ] && brew install ant || true'

However, when applied after if ... fi with a false conditional, I don't think they do anything, and with the true conditional, may mask an error where we want to fail the test. Consider this block:

- if [ "${TRAVIS_CPU_ARCH}" == "arm64" ]; then 
    sudo apt-get install openjdk-11-jdk libltdl-dev;
    export JAVA_HOME=/usr/lib/jvm/java-11-openjdk-arm64;
    export PATH=$JAVA_HOME:$PATH; 
  fi || true

In the case that we aren't on the "arm64" test, we never enter this conditional. The test will not halt after the if [ false ] then ; ... ; fi even without the || true. When we are on arm64, the presence of || true will mask a failure installing openjdk-11, continuing on to later in the script where other problems will occur.

[dbwiddis]

From the bash reference manual regarding an if ... fi:

The return status is the exit status of the last command executed, or zero if no condition tested true.

[dbwiddis]

One more suggestion. I just incorporated arm64 builds on my own project using this as a jumping off point, and occasionally experienced transient problems with the apt install of one of the 80 packages. Recommend adding the -m switch (--fix-missing) to handle more gracefully, e.g., sudo apt-get -m install ...

(Unless that's the whole reason for the || true and I'm finally catching up to where you already were. 😁 )

[odidev]

Updated the travis file as suggested and squashed all changes into one commit with updated commit message. Please review and let me know if any changes required.

Updates:

1. Removed "- sudo: false" command as it is deprecated.
2. Removed "|| true" from if clause in install.
3. Added flag "-m" to apt-get command.
4. Merged if clause under script.
5. Removed JDK 12 job.

[matthiasblaesing]

@odidev looks good to me, though I'd like to wait for @dbwiddis opinion.
One thing that needs fixing is the author information. Please ensure, that your full name is used in the author entry.

[dbwiddis]

Changes look good. I have a few more suggestions after a few more hours playing with my own file, but they aren't really related to this change, so they can either be included or left to a future edit.

• We can simplify the os: declaration to just os: linux. When we explicitly use the include: directive we can add on the osx jobs, but by including that os at the higher level you just create an unneeded osx default JDK job that we later have to exclude. So by simplifying that line you can also remove the exclude: section of the matrix.
• Speaking of matrix:, it's just an alias for jobs: which I find more meaningful when listing specific jobs as we do. That's entirely personal preference, though.
• I had some CI issues with an update to a much older branch, that were fixed by disabling shallow cloning. The default depth is 50 which should work most of the time, particularly on master. But if we ever update the 4.5.2 branch, CI might complain about it. The syntax is:

git:
  depth: false

[odidev]

Updated the travis file as suggested. Please review and let me know if any changes required.

Updates :

1. Added "git.depth" clause.
2. Removed osx from "os: " clause and "exclude: " clause as well.
3. Replaced "matrix: " with "jobs: ".
4. Removed "os: linux" in "jobs" clause and added it above dist as both are related.

@matthiasblaesing : Thanks for the review. Author entry odidev is full name only.

[dbwiddis]

Looks great to me!

[matthiasblaesing]

@odidev I don't understand your reply. I'm accustomed to names consisting of at least a first and a lastname (or given and surname). Of course it is possible, that you come from a culture where only one name is given to a person, but then I'd like to know which. Please accept, that I won't merge without real author information.

You did good work, there is no reason not attach your full name to it.

[odidev]

@matthiasblaesing :

Thanks for the explanation. I work with PureSoftware organization.

Please find below personal details:
Name : Pruthvi Reddy Puresoftware
ID : [email protected]

Please let me know if any more information is required.

[dbwiddis]

@odidev Can you please amend your commit with that info and push it again?
git commit --amend --author="Pruthvi Reddy <[email protected]>"

To permanently set it for all future commits:

git config --global user.name "Pruthvi Reddy"
git config --global user.email "[email protected]"

[odidev]

@dbwiddis : I have updated the author details as suggested.

[matthiasblaesing]

@odidev thank you.

[odidev]

@matthiasblaesing : Thanks for merging the changes. When can we have a release with these changes?

[matthiasblaesing]

@odidev any reason for the rush? The CI job changes nothing for the release. Before and after the change arm64 support is present in the JNA packages.