Updated pipeline for addin/windows build #5
Conversation
|
Hi Nico, This all looks very good! It's a massive change though. Much bigger than I expected. That makes it not so easy to review, also because it's not always very clear what exactly changes in the individual commit. If it is possible to squash certain commits, please don't hesitate to do so as that might make the reviewing easier. On the other hand you may also split up certain commits if that makes sense. I will be adding some comments and questions in the diffs. They may seem dumb questions because I'm not really familiar with the Pipeline 1 code. |
|
Wow, big and useful work @NPavie 👏. I'll review the details ASAP. |
|
small questions @bertfrees @rdeltour : (+ i could try to isolate in a single commit the files updated by com4j, that may be the bigger code changes as i regenerate the bindings with the updated version of com4j) |
|
Hi @bertfrees and @rdeltour , i finished a reorganisation of the commit and their content. 2 commits are still a bit big regarding changes :
|
|
Wonderful @NPavie. That makes it a lot clearer! I will add some small suggestions in the diffs. |
|
The "Win32 UI reintegration" commit needs some more explanation. Perhaps some more words in the README? What exactly is the source code that has been added? It looks like only the stuff in the "bundles" directory has been ported. This seems right because we only need the code that is required to build the Pipeline Lite component. It makes no sense to import the whole thing. However we need to think about whether this is indeed how we want to organize the code, and if so what are the next steps. Presumably the "bundles" directory is also still needed in the daisymfcgui project to build it. But is the code duplication a good idea? |
- removed internal proprietary functions from sun - Added lame 3.1 and AdoptOpenJDK runtimes - updated commons-io, icu4j and com4j libraries - We keep the unzipped version of the dll aside as NullPointerException can be raised by com4j if it fails to unzip its dll. - Updated ant scripts - updated build for pipeline-lite (embedding jre) - Switch source and target java version to 1.8 Also removed unnecessary eclipse build file.
Using correct option --cbr in lame command line call
- Added com4jtools for classes generation - Regenerated speechgen classes - Updated build sapi to use embedded com4j toolset
New pipeline scripts are defined for SaveAsDAISY addin : - The _postprocess script allows to use post-treatment routines on dtbook XML generated by the addin, like cleanups and sentences tagging - The default script converts dtbook xml into DAISY books A new "buildDistForWin" ant target is set to generate the pipeline directory to be included in the addin ressources.
- Merging the repo with the daisy cmf UI svn repository of sourceforge - Preparing maven projects to automate jars rebuild and dependencies for the addin version of the pipeline
Adding informations regarding requirements, core and lite building of the pipeline.
| <include name="_dev/_postprocess.taskScript"/> | ||
| <include name="_dev/default.taskScript"/> |
There was a problem hiding this comment.
maybe we can use more explicit names for these new scripts? (later down the road, when we forget about all this, it may not be obvious that _postprocess and default are made for the AddIn)
I'll update the readme to indicate what was the svn repo holding (the graphical user interface libraries required to launch the pipeline in graphical mode). I had to check back the content of the svn repository because i did not understood what you were talking about with "bundles" directory. i'm not sure to understand what you want exactly or what you mean with "code organization" and "code duplication", and if there are next steps, what are the targetted goals regarding this part of the code base. |
|
If I look at the SVN repo I see a lot of code. The "bundles" directory seems to match what you have imported. So this part of the code lives in two places now. I'm just asking the question whether this is a good idea, whether we need to think more about how to best organize the code. |
- Content of the previous daisymfcgui folder is moved to gui/bundles - Some notes and previous version of code that were kept in the svn repos have not been copied. - Updated ant and maven build for the addin
|
Hi @bertfrees and @rdeltour, i did an update yesterday with the 3 last commits :
I was able to run the code pipeline command line ui class from intellij without any problem (including launching a script), but i have a small problem i'm still working on : To be more clear: But if i try to provide the input for the script, it says it does not find the parameter: (It says |
OK, so what we have now is the whole daisymfcgui repo within the pipeline1 repo. That makes sense I guess. So from now on if someone wants to build or modify the daisymfcgui project, he can checkout the pipeline1 repo. For continuity it would be nice if there were one commit that adds an exact revision of the daisymfcgui repo, and then one or more commits that do additional changes. I've tried to do this. First I added revision 658 and then I compare with your import. I'm seeing more differences than I expected. I have pushed to a513118. @NPavie Can you please check and tell me which differences are intentional and which aren't?
Is there a specific commit that broke things? |
- Switch to java 11 - Updated icu4j to 72_1
Nop, no commit, but checking the bat execution : the "=" is considered as an argument separator by both powershell and the windows CMD terminal ... If i understand your commit, the main differences i see seems to be the changes i made for the addin build using maven instead of the eclipse buildchain. |
- Updated gitignore files - remove the dot folder coming from the old buildchain - pipeline et util libs used by the bundle now injected in the bundles local maven repository
|
So for the "@dot" folder in the commit, it was indeed a relicate of code that came from the previous build chain. I updated the gitignore to avoid commitings libs that are copied during the build process (and are used by the ant process afterward). The core pipeline jars (the pipeline and util jar that are needed in the bundles aggregator) are copied to the local maven repo that is used to keep the eclipse required version of the libs that are not available in maven standard repositories. |
Batch consider the '=' character as a separator when extracting parameters from command line.
|
If i recall, i needed those properties files for the pipeline lite build. |
|
@rdeltour Do you remember how to use git-svn? I've run and waited a day but I'm not sure if it's doing something. |
|
I don’t remember, it’s been a while I haven’t used it. Have you tried standalone |
|
@rdeltour Yes, |
|
This command does seem to work (http instead of https): |
|
I have pushed the migrated repo to https://github.com/daisy/pipeline1/tree/daisymfcgui. |
|
The merge has been done in a5e5b0c. |
|
@NPavie I've rebased the branch and pushed to "dev-npavie-clean". Could you please verify I didn't mess up? Does everything still work? |
|
@bert following christian remarks today (I had completely forgot about the PR) I tested the branch dev-npavie-clean branch and I confirm the build-addin ant file still builds on my side with OpenJDK 11 (even though it's not used any more as pipeline 1 is now replaced by pipeline 2 in SaveAsDAISY) |
|
OK :-) Thanks |
This PR regroup the changes made for the embedded version of the pipeline (pipeline "lite") used in the save as daisy addin.
Most of the changes are targetting the windows UI and the pipeline "lite" created by the "build-addin" ant script.
To test the resulting changes on windows, the build of the pipeline "lite" (in 32 bit version) can be done using
ant -f build-addin.xml.Note that you will need both maven and ant toolchains installed for the build.
Changelog
A dtbook xml post processing script has also been added for the addin, that includes the possibility to tag the sentences.