excelCompare: init at 0.6.1 #55434
excelCompare: init at 0.6.1 #55434
Conversation
ilikeavocadoes
force-pushed
the
excelCompare
branch
from
de1fec9
to
28e8e2b
Compare
| chmod 555 $out/bin/excel_cmp | ||
| mv $out/bin/excel_cmp $out/bin/excel_cmp.sh | ||
| makeWrapper $out/bin/excel_cmp.sh $out/bin/excel_cmp \ | ||
| --prefix PATH : ${lib.makeBinPath [ jdk ]} |
There was a problem hiding this comment.
you don't need to rename excel_cmp before wrapping: wrapProgram is an equivalent of makeWrapper which wraps in place.
But reading the script it would be better to use the approach of https://nixos.org/nixpkgs/manual/#sec-language-java
that is:
- put all jars in $out/share/excel-cmp
- wrap java exactly as they do in the example, setting the class path to $out/share/excel-cmp/*
There was a problem hiding this comment.
Thanks for the review. In the example for makeWrapper, what am I supposed to put in place of org.foo.Main? I tried com.ka.spreadsheet.diff.SpreadSheetDiffer as is in the original excel_cmp startup script, but it results in Error: Could not find or load main class .nix.store.3ykvmyv0ihvdpvxvdcivld3jf89blj83-ExcelCompare-0.6.1.share.excel_cmp.commons-digester-1.8.jar.
There was a problem hiding this comment.
this works for me:
diff --git a/pkgs/tools/text/excelCompare/default.nix b/pkgs/tools/text/excelCompare/default.nix
index a0ec897eb5e..b433aa5e518 100644
--- a/pkgs/tools/text/excelCompare/default.nix
+++ b/pkgs/tools/text/excelCompare/default.nix
@@ -1,4 +1,4 @@
-{stdenv, lib, fetchurl, makeWrapper, unzip, jdk}:
+{stdenv, lib, fetchurl, makeWrapper, unzip, jre}:
stdenv.mkDerivation rec {
name = "ExcelCompare-${version}";
@@ -7,16 +7,14 @@ stdenv.mkDerivation rec {
url = "https://github.com/na-ka-na/ExcelCompare/releases/download/${version}/ExcelCompare-${version}.zip";
sha256 = "1304qraazqm8mgjgnijzl070l0haa8jds7spnsy3xwh3vda0ka4x";
};
- buildInputs = [ jdk unzip makeWrapper ];
+ nativeBuildInputs = [ unzip makeWrapper ];
sourceRoot = ".";
installPhase = ''
- mkdir -p $out
- cp -r bin $out/bin
- chmod 555 $out/bin/excel_cmp
- mv $out/bin/excel_cmp $out/bin/excel_cmp.sh
- makeWrapper $out/bin/excel_cmp.sh $out/bin/excel_cmp \
- --prefix PATH : ${lib.makeBinPath [ jdk ]}
+ install -Dt $out/share/excelcmp bin/dist/*
+ mkdir -p $out/bin
+ makeWrapper ${jre}/bin/java $out/bin/excelcmp \
+ --add-flags "-ea -cp $out/share/excelcmp/\* com.ka.spreadsheet.diff.SpreadSheetDiffer"
'';
meta = {the trick is to prevent the wildcard * from being expanded by the shell
| url = "https://github.com/na-ka-na/ExcelCompare/releases/download/${version}/ExcelCompare-${version}.zip"; | ||
| sha256 = "1304qraazqm8mgjgnijzl070l0haa8jds7spnsy3xwh3vda0ka4x"; | ||
| }; | ||
| buildInputs = [ jdk unzip makeWrapper ]; |
There was a problem hiding this comment.
makewrapper and unzip should be nativeBuildInputs
|
This binary distribution redistributes a number of third party libraries: are they all redistributable under the MIT licence ? |
I will try and find the licenses of the third party libraries. In case they have a different license, what should I put as the |
|
meta.license can be a list, but I don't know if it means license A OR B or A AND B. |
|
Also, please fix the ofborg failure, apparently meta.maintainers does not evaluate. |
Seems like I hadn't already added myself to maintainer list, while I thought I had. I made a pull request of it (#55512). Should I wait until it gets merged and then rebase on master? |
ilikeavocadoes
force-pushed
the
excelCompare
branch
from
28e8e2b
to
b66a76d
Compare
|
I couldn't make sense of the 3rd party licenses, nor how they should be (or should they be) included in the meta attributes, so I just removed that field for the time being. |
|
The source repo has a directory of the third party licenses: https://github.com/na-ka-na/ExcelCompare/tree/master/legal Now the issue seems to be that how can this be included in meta? Also, I'm absolutely clueless about these licenses' implications to the redistributability of this software, as my understanding about the copyright laws is zero. |
|
you can write something like {
meta.license = with stdenv.lib.licenses; [ mit asl20 ];
}I have no idea whether it means OR or AND. |
|
@ilikeavocadoes I would suggest filling an issue upstream to get clarification on the licensing situation so we can proceed with this. |
|
Thank you for your contributions. This has been automatically marked as stale because it has had no activity for 180 days. If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity. Here are suggestions that might help resolve this more quickly:
|
stale
bot
added
the
2.status: stale
|
Closing due to inactivity. Feel free to reopen the discussion. |
Motivation for this change
Add utility to produce text diff of Excel files. Couldn't build from source since it requires Java version <= 6 which is not in Nixpkgs.
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)