07 fixes + tests #147

SimonDanisch · 2018-06-29T16:12:11Z

No description provided.

tkelman · 2018-06-29T16:50:13Z

src/WinRPM.jl

@@ -447,7 +447,13 @@ function do_install(packages::Packages)
    end
 end

-const exe7z = iswindows() ? joinpath(BINDIR, "7z.exe") : "7z"
+const exe7z = if try; success(`7z`) catch; false end;


not a good idea to use any old random version that happens to be on the path, the behavior has been version-specific in ways that have broken this package

tkelman · 2018-06-29T16:50:46Z

src/WinRPM.jl

        (out, pc) = open(cmd, "r")
        stdoutstr = read(out, String)
        if !success(pc)
            wait_close(out)
-            println(stdoutstr)


don't hide error output

Ah, good catch - thought that was one of my debugging println

tkelman · 2018-06-29T16:52:33Z

test/runtests.jl

@@ -0,0 +1,6 @@
+using WinRPM
+using Test
+WinRPM.install("gcc", yes = true)


this has the same problem as #95 (comment) - it should clean up after itself instead of making a mess of the user's environment

SimonDanisch · 2018-06-29T17:31:16Z

Ok, I added a system environment variable for appveyor, so nothing gets installed when anyone runs Pkg.test("WinRPM").

tkelman · 2018-06-29T17:34:39Z

src/WinRPM.jl

@@ -486,7 +486,7 @@ function do_install(package::Package)
                    end
                end
            end
-            isfile(cpio) && rm(cpio)
+            (try; isfile(cpio); catch; false end;) && rm(cpio)


when would this throw?

Good question, it did though... I think this is the whole isfile can throw on windows issue...Maybe it got fixed on latest master. @KristofferC, you're probably up to date on this?

what error, and with what input path?

isfile throws on Windows for url for example, see JuliaLang/julia#26685

when is cpio ever going to have colons? it shouldn't include http:// or things like that

I was just saying when isfile would fail. Whether this ever can happen here I don't know.

tkelman · 2018-06-29T17:36:07Z

Better would be to test in a clean temporary location and clean up after itself, but at least only testing and making a mess on appveyor is better than not testing at all. Would need to be turned on of course.

SimonDanisch · 2018-06-29T17:37:11Z

I turned it on before creating this PR - not sure what's going wrong... -.-

tkelman · 2018-06-30T11:57:20Z

src/WinRPM.jl

@@ -447,7 +447,13 @@ function do_install(packages::Packages)
    end
 end

-const exe7z = iswindows() ? joinpath(BINDIR, "7z.exe") : "7z"
+const exe7z = iswindows()


did you test this locally at all? missing an if

tkelman · 2018-06-30T11:57:43Z

src/WinRPM.jl

-        cpio = splitext(joinpath(cache, escape(basename(path))))[1] * ".cpio"
-    end
-
+    cpio = splitext(path2)[1]*".cpio"


wrong way around, should use the newer code

but that was actually failing for me!

were you using a nightly binary of Julia, or something else? if your code was picking up whatever happened to be on the path, it could easily have been an older version with different behavior

SimonDanisch · 2018-07-02T11:50:28Z

Sorry for being so sloppy, but WinRPM is the last package I want to fix right now ;)

I tested the new version locally and it seems now to work correctly now, even though I rolled back most of my changes.
I'm not sure in what state I tested WinRPM to run into the problems I did - I was definitely running 0.7 since I was testing this with the help of Pk3 + and the new build logs.
It happened before, that Pkg3 forgot about a package I checked out as dev so that it didn't actually pick up any changes.
I hope there isn't just flaky behaviour at work here, since I do definitely remember having builds fail and my fixes did make them succeed...

SimonDanisch · 2018-07-02T11:52:07Z

I did include a last fallback to a system installed 7z, because it's super annoying to fix this when something is wrong with Julia or you just don't want to make win-extras. I added a warning in that case!

tkelman · 2018-07-02T21:12:01Z

src/WinRPM.jl

+    if isfile(_exe7z)
+        _exe7z
+    else # if it's not located in win-extras lets get it catched by the second isfile check
+        joinpath(Sys.BINDIR, "..", "..", "dist-extras", "7z.exe")


If the problem is that you're trying to test packages in source builds on windows, it would be better to fix this in your personal build process rather than copy-paste these fallbacks everywhere.

So do the stuff for cross compiling and creating an installer!?

Or make install and copy files around. Otherwise the environment you're testing in and the environment that practically every Windows user will be working in will have discrepancies.

Hm, I still don't see the exe in the right folder:

sdani@LAPTOP-4CL7JAI3 ~/julia $ make win-extras ... sdani@LAPTOP-4CL7JAI3 ~/julia $ make install /home/sdani/julia/contrib/install.sh 755 /home/sdani/julia/usr/bin/julia /home/s dani/julia/julia-abab96668c/bin/ /home/sdani/julia/contrib/install.sh 755 /home/sdani/julia/usr/bin/*.dll /home/s dani/julia/julia-abab96668c/bin/ /home/sdani/julia/contrib/install.sh 755 /home/sdani/julia/usr/lib/libjulia.dll. a /home/sdani/julia/julia-abab96668c/lib/ /home/sdani/julia/contrib/install.sh 755 /home/sdani/julia/usr/bin/libopenlibm.d ll.a /home/sdani/julia/julia-abab96668c/lib/ # Copy public headers cp -R -L /home/sdani/julia/usr/include/julia/* /home/sdani/julia/julia-abab96668 c/include/julia # Copy system image /home/sdani/julia/contrib/install.sh 755 /home/sdani/julia/usr/lib/julia/sys.dll /home/sdani/julia/julia-abab96668c/lib/julia # Copy in system image build script /home/sdani/julia/contrib/install.sh 755 /home/sdani/julia/contrib/build_sysimg. jl /home/sdani/julia/julia-abab96668c/share/julia/ # Copy in all .jl sources as well cp -R -L /home/sdani/julia/usr/share/julia /home/sdani/julia/julia-abab96668c/sh are/ # Copy documentation cp -R -L /home/sdani/julia/doc/_build/html /home/sdani/julia/julia-abab96668c/sh are/doc/julia/ # Remove various files which should not be installed rm -f /home/sdani/julia/julia-abab96668c/share/julia/base/version_git.sh rm -f /home/sdani/julia/julia-abab96668c/share/julia/test/Makefile # Copy in beautiful new man page /home/sdani/julia/contrib/install.sh 644 /home/sdani/julia/usr/share/man/man1/ju lia.1 /home/sdani/julia/julia-abab96668c/share/man/man1/ # Copy icon and .desktop file mkdir -p /home/sdani/julia/julia-abab96668c/share/icons/hicolor/scalable/apps/ /home/sdani/julia/contrib/install.sh 644 /home/sdani/julia/contrib/julia.svg /ho me/sdani/julia/julia-abab96668c/share/icons/hicolor/scalable/apps/ touch -c /home/sdani/julia/julia-abab96668c/share/icons/hicolor/ gtk-update-icon-cache /home/sdani/julia/julia-abab96668c/share/icons/hicolor/ make: gtk-update-icon-cache: Command not found make: [Makefile:346: install] Error 127 (ignored) mkdir -p /home/sdani/julia/julia-abab96668c/share/applications/ /home/sdani/julia/contrib/install.sh 644 /home/sdani/julia/contrib/julia.desktop /home/sdani/julia/julia-abab96668c/share/applications/ # Install appdata file mkdir -p /home/sdani/julia/julia-abab96668c/share/appdata/ /home/sdani/julia/contrib/install.sh 644 /home/sdani/julia/contrib/julia.appdata .xml /home/sdani/julia/julia-abab96668c/share/appdata/ # Update RPATH entries and JL_SYSTEM_IMAGE_PATH if ../lib/julia != ../lib/julia # On FreeBSD, remove the build's libdir from each library's RPATH mkdir -p /home/sdani/julia/julia-abab96668c/etc cp -R /home/sdani/julia/usr/etc/julia /home/sdani/julia/julia-abab96668c/etc/ sdani@LAPTOP-4CL7JAI3 ~/julia $ usr/bin/julia.exe julia> z7 = joinpath(Sys.BINDIR, "7z.exe") "C:\\Users\\sdani\\juliastuff\\cygwin\\home\\sdani\\julia\\usr\\bin\\7z.exe" julia> isfile(z7) false

Not sure whats up with julia-abab96668c, but even that bin dir doesn't contain a 7z exe.

I'm not going to start copying random files around :D if make win-extras copies it into this folder, this is good enough for me as long as the normal case is covered first. If you have more patience to fix these fiddly issues, i would very much appreciate if you could just push to this PR.

I'm not going to start copying random files around

Why not? That's what building the installer has always done. Guess it happens as part of make binary-dist then. You can locally approximate it. This could be cleaned up in the Julia makefiles, but really isn't that hard to deal with locally - it shouldn't be propagating out into every package that needs to extract files.

please remove this line

davidanthoff · 2018-07-24T19:51:48Z

Bump, what is the status here?

SimonDanisch · 2018-07-24T22:27:30Z

I'm done with this PR and not willing to spend any more time on it ;)
Anyone willing to polish it more / merge it is welcome to do so!

davidanthoff · 2018-07-24T22:39:52Z

Do you think it is ready to be merged? Who can actually merge it?

tkelman · 2018-07-26T20:46:35Z

Should verify that it actually runs and passes on appveyor.

davidanthoff · 2018-07-26T21:00:27Z

Appveyor doesn't seem to be configured for this repo?

I think this PR should no longer be held up by requests for generic new good stuff, like enabling appveyor. If that hasn't been configured for this repo so far, a PR for that would be great, but lets not keep other PRs blocked because of that. We can merge this PR here, and then if someone can configure appveyor, lets merge another PR for that then.

Also, this PR makes master julia 0.7 only, so merging this now won't affect anyone negatively in production, but it has a good chance of making things more workable for folks that are trying to migrate things over to 0.7.

tkelman · 2018-07-26T21:03:38Z

The dist-extras nonsense needs to go away. And some actual evidence that the switch to codeczlib is working correctly is better than "good chance"

tkelman · 2018-07-26T21:05:24Z

test/runtests.jl

+using WinRPM
+using Test
+
+if haskey(ENV, "CI") && haskey(ENV, "WINRPM_DO_THE_TEST") && ENV["WINRPM_DO_THE_TEST"] == "true"


this env var would need to be set in the appveyor config for this to do anything

that should be set in the yaml where it's visible and can be modified by people who don't have access to that settings page - generally the UI should be avoided, the yaml takes precedence for anything set both places

SimonDanisch · 2018-07-28T11:11:21Z

I don't have any time to fiddle around with this PR anymore. @tkelman feel free to take this PR and make the changes you want to see ;)

bicycle1885 and others added 5 commits March 13, 2018 00:20

use CodecZlib.jl instead of Libz.jl

8ca9a05

Merge branch 'codeczlib' of https://github.com/bicycle1885/WinRPM.jl

22d89b2

add tests

fa9ff19

drop 0.6

8f407ab

fixes for 0.7

929e616

SimonDanisch closed this Jun 29, 2018

SimonDanisch reopened this Jun 29, 2018

tkelman reviewed Jun 29, 2018

View reviewed changes

SimonDanisch added 2 commits June 29, 2018 19:29

error printing, no system 7z

b296ffd

don't run any tests on user PC

a6555c6

tkelman reviewed Jun 29, 2018

View reviewed changes

tkelman reviewed Jun 30, 2018

View reviewed changes

fix remaining issues

1865ec5

tkelman reviewed Jul 2, 2018

View reviewed changes

write 0.7 depwarn free code

0e4b318

tkelman reviewed Jul 26, 2018

View reviewed changes

SimonDanisch closed this Jul 27, 2018

SimonDanisch reopened this Jul 27, 2018

SimonDanisch closed this Jul 28, 2018

07 fixes + tests #147

07 fixes + tests #147

Conversation

SimonDanisch commented Jun 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonDanisch commented Jun 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman Jun 29, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman commented Jun 29, 2018

SimonDanisch commented Jun 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonDanisch commented Jul 2, 2018

SimonDanisch commented Jul 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidanthoff commented Jul 24, 2018

SimonDanisch commented Jul 24, 2018

davidanthoff commented Jul 24, 2018

tkelman commented Jul 26, 2018

davidanthoff commented Jul 26, 2018

tkelman commented Jul 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimonDanisch commented Jul 28, 2018

tkelman Jun 29, 2018 •

edited