Re: configure.py regression in RC1 compared to beta
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
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
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
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
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
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
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
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) os.chdir("C:/Users/Usti/AppData/Local/Temp/lyx_tmpdir.ZcmDkPd
Re: configure.py regression in RC1 compared to beta
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
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
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
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
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
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