Bug 1106100 - link-grammar: FTBFS in rawhide
Summary: link-grammar: FTBFS in rawhide
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: link-grammar
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Gwyn Ciesla
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: F21FTBFS
TreeView+ depends on / blocked
 
Reported: 2014-06-09 02:30 UTC by Dennis Gilmore
Modified: 2014-06-30 14:19 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-06-24 18:58:50 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
build.log (1.05 KB, text/plain)
2014-06-09 02:30 UTC, Dennis Gilmore
no flags Details
root.log (73.01 KB, text/plain)
2014-06-09 02:30 UTC, Dennis Gilmore
no flags Details
state.log (945 bytes, text/plain)
2014-06-09 02:30 UTC, Dennis Gilmore
no flags Details
Patch for rawhide (1.72 KB, patch)
2014-06-12 23:54 UTC, Yaakov Selkowitz
no flags Details | Diff

Description Dennis Gilmore 2014-06-09 02:30:17 UTC
Your package link-grammar failed to build from source in current rawhide.

http://koji.fedoraproject.org/koji/taskinfo?taskID=6948688

For details on mass rebuild see https://fedoraproject.org/wiki/Fedora_21_Mass_Rebuild

Comment 1 Dennis Gilmore 2014-06-09 02:30:19 UTC
Created attachment 903883 [details]
build.log

Comment 2 Dennis Gilmore 2014-06-09 02:30:21 UTC
Created attachment 903886 [details]
root.log

Comment 3 Dennis Gilmore 2014-06-09 02:30:22 UTC
Created attachment 903887 [details]
state.log

Comment 4 Yaakov Selkowitz 2014-06-12 23:54:11 UTC
Created attachment 908334 [details]
Patch for rawhide

Cause was an obsolete BR: libgcj-devel (gcj was removed from gcc-4.9) but was only used to provide a jni.h that configure would find by itself.  Fortunately there was an easy workaround; patch for rawhide attached.

Comment 5 Gwyn Ciesla 2014-06-24 18:58:50 UTC
Excellent, I've incorporated your changes into my update to 5.0.8.  Thanks very much!

Comment 6 Adam Williamson 2014-06-27 18:08:35 UTC
unfortunately, abiword does not build against link-grammar 5.x, so now we have an abiword package with broken deps which breaks quite a lot of live image builds.

See https://bugzilla.redhat.com/show_bug.cgi?id=1113457 , http://bugzilla.abisource.com/show_bug.cgi?id=13650 .

Comment 7 Gwyn Ciesla 2014-06-27 18:14:16 UTC
My recent email to the abiword maintainer:

I've managed a patch to get it to build, because there were incompatible API changes, and I *think* I got it right, but I don't know.  I also don't have a working rawhide system or VM to test it.  Do you?  The patch is:

--- abiword-3.0.0/plugins/grammar/linkgrammarwrap/LinkGrammarWrap.cpp.orig      2013-04-07 08:53:03.000000000 -0500
+++ abiword-3.0.0/plugins/grammar/linkgrammarwrap/LinkGrammarWrap.cpp   2014-06-27 08:58:42.186089562 -0500
@@ -145,11 +145,11 @@
          }
          AbiGrammarError * pWordMap = new  AbiGrammarError();
          pWordMap->m_iErrLow = iLow;
-         pWordMap->m_iErrHigh = iLow + strlen(sentence_get_nth_word(sent, i));
+         pWordMap->m_iErrHigh = iLow + sentence_link_cost(sent, i);
          pWordMap->m_iWordNum = i;
          vecMapOfWords.addItem(pWordMap);
          bool bNew = false;
-         if(!sentence_nth_word_has_disjunction(sent, i))
+         if(!sentence_disjunct_cost(sent, i))
          {
            //UT_DEBUGMSG(("|%s| NULL LINK\n",sent->word[i].string));
            if(pErr == NULL)
@@ -167,7 +167,7 @@
                }
                pErr = new AbiGrammarError();
              }
-             iHigh = iLow + strlen(sentence_get_nth_word(sent, i));
+             iHigh = iLow + sentence_link_cost(sent, i);
              pErr->m_iErrLow = iLow + iOff -1;
              pErr->m_iErrHigh = iHigh + iOff -1;
              if(pErr->m_iErrLow < 0)
@@ -188,7 +188,7 @@
              //
              // Expand the sqiggle
              //
-             iHigh = iLow + strlen(sentence_get_nth_word(sent, i)) + iOff;
+             iHigh = iLow + sentence_link_cost(sent, i) + iOff;
              pErr->m_iErrHigh = iHigh;
              if(pErr->m_iErrHigh < totlen-1)
              {
@@ -197,7 +197,7 @@
              pErr->m_iWordNum = i;
            }
          }
-         iLow += strlen(sentence_get_nth_word(sent, i));
+         iLow += sentence_link_cost(sent, i);
        }
        //
        // No NULL links but still an error , mark the whole sentence bad.

Comment 8 Gwyn Ciesla 2014-06-27 18:15:00 UTC
I can commit and build but I can't test.

Comment 9 Adam Williamson 2014-06-27 18:16:53 UTC
given that abiword will not run at present and our live composes are busted, it's probably worth the risk. I have a working Rawhide system and can test, if you let me know exactly what should be tested to make you happy the fix is good - just enable the grammar checker and load in a document, or something?

I guess you could also post the patch to my upstream bug :)

Comment 10 Gwyn Ciesla 2014-06-27 18:21:40 UTC
It think that would be sufficient.  I'll post a link to the koji build when complete.

Comment 11 David Tardon 2014-06-27 18:51:24 UTC
(In reply to Jon Ciesla from comment #7)
> My recent email to the abiword maintainer:

Ah, but I am not an abiword maintainer... Or, if I am, I am not aware of it =) You want Marc Maurer, a.k.a. uwog (uwog)...

Comment 12 Gwyn Ciesla 2014-06-27 18:57:14 UTC
I see.  CCing.

Comment 13 Adam Williamson 2014-06-27 19:24:39 UTC
jean on #abiword IRC suggested:

 anyway the patch at least needs to also check for the new function

so, I went poking through old versions (god, how did we live without git blame? this is horrible) and it appears that both sentence_link_cost() and sentence_disjunct_cost() exist as far back as 4.3.5. sentence_link_cost() does not exist in 4.3.4, so that would be the newest version you could no longer build abiword against, if this patch were added as-is.

Comment 14 Adam Williamson 2014-06-27 19:30:27 UTC
looks like the patch should change:

grammar_pkgs='link-grammar >= 4.2.1'

to:

grammar_pkgs='link-grammar >= 4.3.5'

Comment 15 Gwyn Ciesla 2014-06-27 19:52:26 UTC
intel builds are done, waiting on arm:

http://koji.fedoraproject.org/koji/taskinfo?taskID=7084546

If this works, feel free to add #14 to the patch, or I can.

Comment 16 Gwyn Ciesla 2014-06-27 19:52:43 UTC
And arm is done.  There's timing for you.

Comment 17 Adam Williamson 2014-06-27 20:19:33 UTC
looks good here - built abiword launches, and underlines things sensibly if I enable on-the-fly grammar checking. It doesn't suggest corrections, but then it looks like the older build didn't either, so I think that's just missing functionality in AbiWord, not a new problem caused by the change.

Comment 18 Linas Vepstas 2014-06-27 22:10:19 UTC
Hi John,

I'm the link-grammar maintainer; I hate to say it, but the patch in comment #7 is wrong ...

If I may, I'd like to describe the proper fix, rather than provide it?  (I don't have the abi source at my fingertips, and have never even tried to compile it...)

Perhaps I should open a new bug for this?

circa original line 147, replace sentence_get_nth_word(sent, i) with the following:

const char * sentence_get_nth_word(Sentence sent, int w) {
   Parse_Options opts = parse_options_create();
   Linkage lkg = linkage_create(0, sent, opts);
   const char* word =  linkage_get_word(lkg, w);
   linkage_delete(lkg);
   parse_options_delete(opts);
   return word;
}

If there's a copy of Parse_Options floating around somewhere, use that, instead of mallocing and deleting each time.  It would also be much better to move the Linkage lkg = linkage_create(0, sent, opts); outside of the whatever the loop is that is looping over i. (for performance reasons).   The other calls to sentence_get_nth_word()'s would get replaced likewise.
 

Then circa line 151, replace sentence_nth_word_has_disjunction with "false", so that the argument to the if() is always true, the if is always taken.

That's it.

BTW, using sentence_link_cost() and sentence_disjunct_cost() as used above will almost surely trigger an assert somewhere in the code. (Unless I did something "friendly" e.g. remove the assert and return garbage. Halt and catch fire. These are utterly unrelated routines, and the int args the take are not word numbers.

Will try followup w/ email.

Comment 19 Yaakov Selkowitz 2014-06-27 22:25:26 UTC
link-grammar is only used by abiword and kdepim.  Maybe it's better to revert to latest link-grammar-4.x for now?

Comment 20 Adam Williamson 2014-06-27 22:33:16 UTC
linas: there's an RH bug for the abiword compile fail at https://bugzilla.redhat.com/show_bug.cgi?id=1113457 , and an upstream (abisource) bug at http://bugzilla.abisource.com/show_bug.cgi?id=13650 - the latter is probably the best place.

Comment 21 Linas Vepstas 2014-06-28 00:34:47 UTC
The last 4.x is link-grammar-4.8.6 from February 2014, its a good choice. I've not gotten any bug reports on it.

The new version 5 series is newer and shinier, but actually rather unstable at the moment; big changes are ongoing.

Comment 22 Adam Williamson 2014-06-28 00:36:30 UTC
the package changelog kind of implies 5.x was chosen because 4.x didn't build any more, but I'm not sure if that's correct...Jon?

Comment 23 Linas Vepstas 2014-06-28 00:50:00 UTC
Yaakov,  the rpm spec badly mis-characterizes link-grammar. It is not "A Grammar Checking library", but a full-service natural language dependency parser for English and Russian, with prototypes for other assorted languages. Its like NLTK, or the Stanford parser, but more powerful, with broader coverage.

Comment 24 Yaakov Selkowitz 2014-06-29 05:03:08 UTC
(In reply to Linas Vepstas from comment #23)
> Yaakov,  the rpm spec badly mis-characterizes link-grammar.

This would be a (separate) issue for Jon, the assignee of this bug.

Comment 25 Linas Vepstas 2014-06-29 19:58:23 UTC
Here's the corrected patch that I'm sending upstream

--- abiword-3.0.0/plugins/grammar/linkgrammarwrap/LinkGrammarWrap.cpp.orig	2014-06-29 14:50:30.000000000 -0500
+++ abiword-3.0.0/plugins/grammar/linkgrammarwrap/LinkGrammarWrap.cpp	2014-06-29 14:55:39.000000000 -0500
@@ -145,12 +145,11 @@ bool LinkGrammarWrap::parseSentence(Piec
 	  }
 	  AbiGrammarError * pWordMap = new  AbiGrammarError();
 	  pWordMap->m_iErrLow = iLow;
-	  pWordMap->m_iErrHigh = iLow + strlen(sentence_get_nth_word(sent, i));
+	  pWordMap->m_iErrHigh = iLow + strlen(linkage_get_word(linkage, i));
 	  pWordMap->m_iWordNum = i;
 	  vecMapOfWords.addItem(pWordMap);
 	  bool bNew = false;
-	  if(!sentence_nth_word_has_disjunction(sent, i))
-	  {
+
 	    //UT_DEBUGMSG(("|%s| NULL LINK\n",sent->word[i].string));
 	    if(pErr == NULL)
 	    {
@@ -167,7 +166,7 @@ bool LinkGrammarWrap::parseSentence(Piec
 		}
 		pErr = new AbiGrammarError();
 	      }
-	      iHigh = iLow + strlen(sentence_get_nth_word(sent, i));
+	      iHigh = iLow + strlen(linakge_get_word(linkage, i));
 	      pErr->m_iErrLow = iLow + iOff -1;
 	      pErr->m_iErrHigh = iHigh + iOff -1;
 	      if(pErr->m_iErrLow < 0)
@@ -188,7 +187,7 @@ bool LinkGrammarWrap::parseSentence(Piec
 	      //
 	      // Expand the sqiggle
 	      //
-	      iHigh = iLow + strlen(sentence_get_nth_word(sent, i)) + iOff;
+	      iHigh = iLow + strlen(linkage_get_word(linkage, i)) + iOff;
 	      pErr->m_iErrHigh = iHigh;
 	      if(pErr->m_iErrHigh < totlen-1)
 	      {
@@ -196,8 +195,7 @@ bool LinkGrammarWrap::parseSentence(Piec
 	      }
 	      pErr->m_iWordNum = i;
 	    }
-	  }
-	  iLow += strlen(sentence_get_nth_word(sent, i));
+	  iLow += strlen(linkage_get_word(linkage, i));
 	}
 	//
 	// No NULL links but still an error , mark the whole sentence bad.

Comment 26 Gwyn Ciesla 2014-06-30 13:20:28 UTC
Linas, thanks for the patch, I'll update rawhide with it.  I'll also re-work the description.

Adam, actually I was able to get 4.x to build with Yaakov's patch but I thought 5.x would be a better choice for rawhide, to increase it's use and drive feedback, bug reporting, etc.  I also wasn't aware it wasn't considered stable.

Comment 27 Linas Vepstas 2014-06-30 13:55:24 UTC
I posted a second patch at https://bugzilla.redhat.com/show_bug.cgi?id=1113457 to handle the remaining warnings.

All link-grammar point releases are supposed to be "stable".  Which doesn't mean we don't make mistakes. There are some major changes in the 5.0 series, and they're still coming in, so I expect some turbulence, hopefully invisible to the outside world.  There will be a 5.1.0 in a week or two.

There's nothing wrong with using the LG 5.x series in rawhide.

Comment 28 Linas Vepstas 2014-06-30 13:58:40 UTC
I've just been told abiword upstream svn now has these patches applied.

Comment 29 Gwyn Ciesla 2014-06-30 14:13:59 UTC
FYI, there's a typo in the second instance of linkage_get_word.

Comment 30 Gwyn Ciesla 2014-06-30 14:19:19 UTC
Ok, I'll apply that as well.


Note You need to log in before you can comment on or make changes to this bug.