Re: configure.py regression in RC1 compared to beta

[Scott Kostyshak]

On Tue, Oct 31, 2017 at 09:19:49AM +, Stephan Witt wrote:
> Am 30.10.2017 um 08:31 schrieb Scott Kostyshak :
> > 
> > On Sun, Oct 29, 2017 at 09:10:23PM +, Uwe Stöhr wrote:
> >> El 29.10.2017 a las 16:52, Jürgen Spitzmüller escribió:
> >> 
> >>> Note that I am traveling. So if it works on the Mac, someone will have
> >>> to cherry-pick it.
> >> 
> >> I might also not be able to commit soon. So Scott, could you please do this
> >> if Stephan gave his OK?
> > 
> > Yes I can take care of that once Stephan has a chance to test.
> 
> I did the test with a checkout of 2.3.x and the cherry-picked change
> 22125fa6de76ec884a0a0c41506e78a8fdae575c and I tested it with and w/o
> inkscape and it works for me on Mac. Thank you, Jürgen.

Thanks for testing, Stephan. I'm glad we got this figured out.

I've cherry-picked and pushed the commits. I'll send the new tars in a
bit.

Scott

> Stephan
>
> > Thanks a lot, Jürgen and Uwe for figuring out a fix.


signature.asc
Description: PGP signature

Re: configure.py regression in RC1 compared to beta

[Stephan Witt]

Am 30.10.2017 um 08:31 schrieb Scott Kostyshak :
> 
> On Sun, Oct 29, 2017 at 09:10:23PM +, Uwe Stöhr wrote:
>> El 29.10.2017 a las 16:52, Jürgen Spitzmüller escribió:
>> 
>>> Note that I am traveling. So if it works on the Mac, someone will have
>>> to cherry-pick it.
>> 
>> I might also not be able to commit soon. So Scott, could you please do this
>> if Stephan gave his OK?
> 
> Yes I can take care of that once Stephan has a chance to test.

I did the test with a checkout of 2.3.x and the cherry-picked change
22125fa6de76ec884a0a0c41506e78a8fdae575c and I tested it with and w/o
inkscape and it works for me on Mac. Thank you, Jürgen.

Stephan

> Thanks a lot, Jürgen and Uwe for figuring out a fix.
> 
>> Moreover, I propose to commit configure.py as it is in master to 2.3 because
>> it also had some improvements LyX 2.3 should benefit from.
> 
> I'll cherry-pick Jürgen's change regarding WMF -> EPS converter. I don't
> want the commit that stores text editors in a variable in RC1 (even
> though I requested that improvement) since it is more of a long-run
> benefit fix. If there's another commit regarding configure.py that you
> think should be backported, let me know. We have to be careful because
> we want compatibility between different Python versions, and even small
> changes that appear harmless can break compatibility that we might not
> catch.
> 
> Scott

Re: configure.py regression in RC1 compared to beta

[Uwe Stöhr]

El 30.10.2017 a las 08:31, Scott Kostyshak escribió:


I'll cherry-pick Jürgen's change regarding WMF -> EPS converter. I don't
want the commit that stores text editors in a variable in RC1 (even
though I requested that improvement) since it is more of a long-run
benefit fix.


OK, then only also backport the wmf2eps removal.

thanks and regards
Uwe

Re: configure.py regression in RC1 compared to beta

[Scott Kostyshak]

On Sun, Oct 29, 2017 at 09:10:23PM +, Uwe Stöhr wrote:
> El 29.10.2017 a las 16:52, Jürgen Spitzmüller escribió:
> 
> > Note that I am traveling. So if it works on the Mac, someone will have
> > to cherry-pick it.
> 
> I might also not be able to commit soon. So Scott, could you please do this
> if Stephan gave his OK?

Yes I can take care of that once Stephan has a chance to test.

Thanks a lot, Jürgen and Uwe for figuring out a fix.

> Moreover, I propose to commit configure.py as it is in master to 2.3 because
> it also had some improvements LyX 2.3 should benefit from.

I'll cherry-pick Jürgen's change regarding WMF -> EPS converter. I don't
want the commit that stores text editors in a variable in RC1 (even
though I requested that improvement) since it is more of a long-run
benefit fix. If there's another commit regarding configure.py that you
think should be backported, let me know. We have to be careful because
we want compatibility between different Python versions, and even small
changes that appear harmless can break compatibility that we might not
catch.

Scott


signature.asc
Description: PGP signature

Re: configure.py regression in RC1 compared to beta

[Uwe Stöhr]

El 29.10.2017 a las 16:52, Jürgen Spitzmüller escribió:


Note that I am traveling. So if it works on the Mac, someone will have
to cherry-pick it.


I might also not be able to commit soon. So Scott, could you please do 
this if Stephan gave his OK?


Moreover, I propose to commit configure.py as it is in master to 2.3 
because it also had some improvements LyX 2.3 should benefit from.


regards Uwe

Re: configure.py regression in RC1 compared to beta

[Jürgen Spitzmüller]

Am Sonntag, den 29.10.2017, 15:17 +0100 schrieb Uwe Stöhr:
> El 29.10.2017 a las 08:54, Jürgen Spitzmüller escribió:
> 
> > > Any thoughts on whether this should be an rc1 blocker?
> > 
> > Yes, it should, if only since we need the rc to test all OSes.
> 
> +1
> 
> > Anyway, I have pushed a fix to master. It works on Linux.
> 
> Great! This works here.

Note that I am traveling. So if it works on the Mac, someone will have
to cherry-pick it.

Jürgen

> 
> best regards
> Uwe

signature.asc
Description: This is a digitally signed message part

Re: configure.py regression in RC1 compared to beta

[Uwe Stöhr]

El 29.10.2017 a las 08:54, Jürgen Spitzmüller escribió:


Any thoughts on whether this should be an rc1 blocker?


Yes, it should, if only since we need the rc to test all OSes.


+1


Anyway, I have pushed a fix to master. It works on Linux.


Great! This works here.

best regards
Uwe

Re: configure.py regression in RC1 compared to beta

[Uwe Stöhr]

El 28.10.2017 a las 19:25, Jürgen Spitzmüller escribió:


It should be quoted. You can check via the messages pane.


Hi Jürgen,

sorry I did not have time to look closer up to now.
Nevertheless I cannot see the complete path behind $$p, all I get is this:

---
from_file:D:/lyxdokumente/GraphicsTest/germany.emf
	to_file_base: 
C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/CacheItem.FS4192

from_format:  emf
to_format:png
graphics/GraphicsConverter.cpp (287): build_script ...
graphics/GraphicsConverter.cpp (414): ready!
graphics/GraphicsConverter.cpp (155):   Conversion script:
--
#!/usr/bin/env python
# -*- coding: utf-8 -*-
import os, shutil, sys

def unlinkNoThrow(file):
  ''' remove a file, do not throw if an error occurs '''
  try:
os.unlink(file)
  except:
pass

infile = "D:/lyxdokumente/GraphicsTest/germany.emf"
outfile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192.emf"

shutil.copy(infile, outfile)
os.chdir("C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/")
infile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192.emf"
infile_base = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192"
outfile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192.eps"

outdir  = os.path.dirname(outfile)

if os.system(r'inkscape --file=$$p' + '"' + infile + '"' + ' 
--export-area-drawing --without-gui --export-eps=$$p' + '"' + outfile + 
'"' + '') != 0:

  unlinkNoThrow(outfile)
  sys.exit(1)

if not os.path.isfile(outfile):
  if os.path.isfile(outfile + '.0'):
os.rename(outfile + '.0', outfile)
import glob
for file in glob.glob(outfile + '.?'):
  unlinkNoThrow(file)
  else:
sys.exit(1)

if infile != outfile:
  unlinkNoThrow(infile)

infile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192.eps"
infile_base = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192"
outfile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192.png"

outdir  = os.path.dirname(outfile)
...
---

But

infile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192.eps"


does not exist. So Inkscape did not create an EPS out of the EMF and 
this is how I found the reason with the $$p in the path to Inkscape.


I am unable to see what is behind the $$p variable. I mean if I omit the 
$$p variable as I did in my path for configure.py I get:


---
from_file:D:/lyxdokumente/GraphicsTest/germany.emf
	to_file_base: 
C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/CacheItem.ji4192

from_format:  emf
to_format:png
graphics/GraphicsConverter.cpp (287): build_script ...
graphics/GraphicsConverter.cpp (414): ready!
graphics/GraphicsConverter.cpp (155):   Conversion script:
--
#!/usr/bin/env python
# -*- coding: utf-8 -*-
import os, shutil, sys

def unlinkNoThrow(file):
  ''' remove a file, do not throw if an error occurs '''
  try:
os.unlink(file)
  except:
pass

infile = "D:/lyxdokumente/GraphicsTest/germany.emf"
outfile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertHU4192.emf"

shutil.copy(infile, outfile)
os.chdir("C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/")
infile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertHU4192.emf"
infile_base = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertHU4192"
outfile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertHU4192.eps"

outdir  = os.path.dirname(outfile)

if os.system(r'inkscape --file=' + '"' + infile + '"' + ' 
--export-area-drawing --without-gui --export-eps=' + '"' + outfile + '"' 
+ '') != 0:

  unlinkNoThrow(outfile)
  sys.exit(1)

if not os.path.isfile(outfile):
  if os.path.isfile(outfile + '.0'):
os.rename(outfile + '.0', outfile)
import glob
for file in glob.glob(outfile + '.?'):
  unlinkNoThrow(file)
  else:
sys.exit(1)

if infile != outfile:
  unlinkNoThrow(infile)

infile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertHU4192.eps"
infile_base = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertHU4192"
outfile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertHU4192.png"

outdir  = os.path.dirname(outfile)
---

So to compare the relevant part we have:

- with $$p:

infile = "D:/lyxdokumente/GraphicsTest/germany.emf"
outfile = 
"C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPdt4192/gconvertRR4192.emf"

shutil.copy(infile, outfile)

Re: configure.py regression in RC1 compared to beta

[Stephan Witt]

Am 29.10.2017 um 08:54 schrieb Jürgen Spitzmüller :
> 
> Am Samstag, den 28.10.2017, 15:59 -0400 schrieb Scott Kostyshak:
>> I think that this is the type of issue that would not necessarily be
>> a
>> beta-blocker but that should be an rc-blocker. Indeed, we prioritized
>> fixing SVG conversion on Mac before rc1, which we did; so I suppose
>> we
>> should consider this as an rc1-blocker.
>> 
>> Any thoughts on whether this should be an rc1 blocker?
> 
> Yes, it should, if only since we need the rc to test all OSes.
> 
> Anyway, I have pushed a fix to master. It works on Linux.
> 
> Uwe, Stephan, can you test on your OSes, please?

Jürgen,

many thanks for doing this. I’m away from home with friends and don’t have time 
to answer ATM.

In case of a short break I’ll test and report back.

Stephan

Re: configure.py regression in RC1 compared to beta

[Jürgen Spitzmüller]

Am Samstag, den 28.10.2017, 15:59 -0400 schrieb Scott Kostyshak:
> I think that this is the type of issue that would not necessarily be
> a
> beta-blocker but that should be an rc-blocker. Indeed, we prioritized
> fixing SVG conversion on Mac before rc1, which we did; so I suppose
> we
> should consider this as an rc1-blocker.
> 
> Any thoughts on whether this should be an rc1 blocker?

Yes, it should, if only since we need the rc to test all OSes.

Anyway, I have pushed a fix to master. It works on Linux.

Uwe, Stephan, can you test on your OSes, please?

Jürgen


> 
> Scott

signature.asc
Description: This is a digitally signed message part

Re: configure.py regression in RC1 compared to beta

[Scott Kostyshak]

On Sat, Oct 28, 2017 at 05:25:46PM +, Jürgen Spitzmüller wrote:
> Am Samstag, den 28.10.2017, 18:49 +0200 schrieb Uwe Stöhr:
> > El 28.10.2017 a las 14:05, Jürgen Spitzmüller escribió:
> > 
> > > > How can we fix this?
> > > 
> > > If it turns out that Inkscape on Windows cannot deal with paths we
> > > need
> > > to introduce a function for configure.py that returns full path or
> > > only
> > > file name depending on the OS.
> > 
> > I think this is the problem as far as I investigated now.
> > OK, I was not yet able to check if the path behind $$p is quoted or 
> > note. If not this would explain why it fails on Windows.
> 
> It should be quoted. You can check via the messages pane.

So this issue breaks conversion of SVG, EMF, and WMF images for all
Windows users? That is concerning.

I think that many would be able to test without running into this issue
because most users do not have SVG, EMF, and WMF images in their
documents; but I think that many also do have them. Further, it is bad
that users will get errors when compiling the manuals.

I think that this is the type of issue that would not necessarily be a
beta-blocker but that should be an rc-blocker. Indeed, we prioritized
fixing SVG conversion on Mac before rc1, which we did; so I suppose we
should consider this as an rc1-blocker.

Any thoughts on whether this should be an rc1 blocker?

Scott


signature.asc
Description: PGP signature

Re: configure.py regression in RC1 compared to beta

[Jürgen Spitzmüller]

Am Samstag, den 28.10.2017, 18:49 +0200 schrieb Uwe Stöhr:
> El 28.10.2017 a las 14:05, Jürgen Spitzmüller escribió:
> 
> > > How can we fix this?
> > 
> > If it turns out that Inkscape on Windows cannot deal with paths we
> > need
> > to introduce a function for configure.py that returns full path or
> > only
> > file name depending on the OS.
> 
> I think this is the problem as far as I investigated now.
> OK, I was not yet able to check if the path behind $$p is quoted or 
> note. If not this would explain why it fails on Windows.

It should be quoted. You can check via the messages pane.

Jürgen

> 
> regards Uwe

signature.asc
Description: This is a digitally signed message part

Re: configure.py regression in RC1 compared to beta

[Uwe Stöhr]

El 28.10.2017 a las 14:05, Jürgen Spitzmüller escribió:


How can we fix this?


If it turns out that Inkscape on Windows cannot deal with paths we need
to introduce a function for configure.py that returns full path or only
file name depending on the OS.


I think this is the problem as far as I investigated now.
OK, I was not yet able to check if the path behind $$p is quoted or 
note. If not this would explain why it fails on Windows.


regards Uwe

Re: configure.py regression in RC1 compared to beta

[Jürgen Spitzmüller]

Am Samstag, den 28.10.2017, 13:08 +0200 schrieb Uwe Stöhr:
> Hi Stephan,
> 
> With LyX 2.3.0beta1 I can convert EMF images in LyX file while this 
> fails in RC1.

Can you please elaborate what exactly fails?

> The reason is commit
> http://www.lyx.org/trac/changeset/350ef993/lyxgit
> 
> The bug is that the path must not be output for inkscape on Win and 
> apparently also not on Linux. 

Yes, the path is not necessary on Linux, but it also does not harm
here.

> As this seems to be a special thing for 
> MacOS, we must only do this on Mac.
> 
> Attached is a patch to restore the working behavior before your
> commit.
> 
> 
> Note that you did not make your change in line
> 'a SVG -> PNG converter'
> To be consistent also this inkscape call should have the $$p call.
> 
> How can we fix this?

If it turns out that Inkscape on Windows cannot deal with paths we need
to introduce a function for configure.py that returns full path or only
file name depending on the OS.

But first, I'd like to know what actually is the problem.

Jürgen

> 
> thanks and regards
> Uwe
> 

signature.asc
Description: This is a digitally signed message part