Discussion:
[Freecol-developers] patch: fix looking for jar in FHS location
Michael Vetter
2016-09-20 09:37:19 UTC
Permalink
Hello guys,

I think I spotted a small mistake in the freecol.sh script.
You use test -d there which is used to look for a directory. Actually
we want to look for a jar file in that directory, so we should use test
-r.

Let me know what you think.
Patch is attached to this mail.

jubalh
Michael T. Pope
2016-09-20 10:15:51 UTC
Permalink
On Tue, 20 Sep 2016 11:37:19 +0200
Post by Michael Vetter
I think I spotted a small mistake in the freecol.sh script.
You use test -d there which is used to look for a directory. Actually
we want to look for a jar file in that directory, so we should use test
-r.
Good catch. Applied.

commit d74dff90cb357ceeec3326dc2910d8eb6a5fe95a
Author: Mike Pope <***@computer.org>
Date: Tue Sep 20 19:43:54 2016 +0930

Fix for jar finding in freecol.sh, from Michael Vetter.

Cheers,
Mike Pope
Michael Vetter
2016-09-20 10:52:10 UTC
Permalink
On Tue, 20 Sep 2016 19:45:51 +0930
Post by Michael T. Pope
On Tue, 20 Sep 2016 11:37:19 +0200
Post by Michael Vetter
I think I spotted a small mistake in the freecol.sh script.
You use test -d there which is used to look for a directory.
Actually we want to look for a jar file in that directory, so we
should use test -r.
Good catch. Applied.
:-)

Oh but you made a small mistake.
If you would have used: git apply
0001-Fix-testing-of-jar-in-FHS-location.patch

Thenn all the author information would have been used. Like this it was
written like its your patch. Some people could either take this the
wrong way (unfair) or generally it helps with git blame if you have
this info :-)

------------------------------------------------------------------------------
Caleb Williams
2016-09-21 02:34:37 UTC
Permalink
Post by Michael Vetter
Oh but you made a small mistake.
If you would have used: git apply
0001-Fix-testing-of-jar-in-FHS-location.patch
Thenn all the author information would have been used. Like this it was
written like its your patch. Some people could either take this the
wrong way (unfair) or generally it helps with git blame if you have
this info :-)
Mike,

Can we revert your commit and re-apply with git apply? Does SourceForge
support this feature?

I'll give it a try at least.

Thanks,

Caleb
Caleb Williams
2016-09-21 02:43:07 UTC
Permalink
Post by Caleb Williams
Post by Michael Vetter
Oh but you made a small mistake.
If you would have used: git apply
0001-Fix-testing-of-jar-in-FHS-location.patch
Thenn all the author information would have been used. Like this it was
written like its your patch. Some people could either take this the
wrong way (unfair) or generally it helps with git blame if you have
this info :-)
Mike,
Can we revert your commit and re-apply with git apply? Does SourceForge
support this feature?
I'll give it a try at least.
Thanks,
Caleb
Gave it a shot and got an error message as follows:

***@caleb-laptop ~/Desktop/FreeCol Repository/freecol (master)
$ git apply 0001-Fix-testing-of-jar-in-FHS-location.patch
0001-Fix-testing-of-jar-in-FHS-location.patch:21: trailing whitespace.
elif test -r "/usr/share/java/${FREECOLJAR}" ; then
warning: packaging/common/freecol.sh has type 100644, expected 100755
error: patch failed: packaging/common/freecol.sh:66
error: packaging/common/freecol.sh: patch does not apply

Can you give it a shot Mike?

Thanks,

- Caleb
Michael T. Pope
2016-09-21 03:54:41 UTC
Permalink
On Tue, 20 Sep 2016 21:43:07 -0500
Post by Caleb Williams
Can you give it a shot Mike?
Michael Vetter has a point about the information being richer, and
somewhat politer, but cycling the repo for a one character fix is
disproportionate.

Cheers,
Mike Pope
Caleb Williams
2016-09-21 04:25:27 UTC
Permalink
I agree from a practical standpoint 100%. If we can share the credit I'm
all for that from a principle standpoint.

If SF can't make it work, then we have no choice but to commit the change
ourselves.

Caleb
Post by Michael T. Pope
On Tue, 20 Sep 2016 21:43:07 -0500
Post by Caleb Williams
Can you give it a shot Mike?
Michael Vetter has a point about the information being richer, and
somewhat politer, but cycling the repo for a one character fix is
disproportionate.
Cheers,
Mike Pope
------------------------------------------------------------
------------------
_______________________________________________
Freecol-developers mailing list
https://lists.sourceforge.net/lists/listinfo/freecol-developers
Michael Vetter
2016-09-21 14:03:07 UTC
Permalink
On Tue, 20 Sep 2016 23:25:27 -0500
Post by Caleb Williams
I agree from a practical standpoint 100%. If we can share the credit
I'm all for that from a principle standpoint.
If SF can't make it work, then we have no choice but to commit the
change ourselves.
No, don't worry guys, its not that important :-)

Just wanted to mention it for the next time.

Michael

------------------------------------------------------------------------------
Caleb Williams
2016-09-21 22:01:57 UTC
Permalink
Okay, I put the fix back in.

Thanks again Michael for your contribution.

- Caleb

Loading...