Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filenames not added to indexes #82

Closed
piranna opened this issue Oct 13, 2015 · 4 comments
Closed

Filenames not added to indexes #82

piranna opened this issue Oct 13, 2015 · 4 comments

Comments

@piranna
Copy link
Contributor

piranna commented Oct 13, 2015

When using applyPatches() with https://raw.githubusercontent.com/GregorR/musl-cross/master/patches/gcc-4.7.3-musl.diff I get the next result on loadFile callback:

{ hunks: 
   [ { oldStart: 264,
       oldLines: 6,
       newStart: 264,
       newLines: 13,
       lines: [Object] },
     { oldStart: 272,
       oldLines: 6,
       newStart: 279,
       newLines: 9,
       lines: [Object] },
     { oldStart: 522,
       oldLines: 7,
       newStart: 522,
       newLines: 7,
       lines: [Object] },
     { oldStart: 625,
       oldLines: 6,
       newStart: 625,
       newLines: 9,
       lines: [Object] },
     { oldStart: 33,
       oldLines: 10,
       newStart: 33,
       newLines: 12,
       lines: [Object] },
     { oldStart: 54,
       oldLines: 18,
       newStart: 56,
       newLines: 21,
       lines: [Object] },
     { oldStart: 85,
       oldLines: 21,
       newStart: 90,
       newLines: 21,
       lines: [Object] },
     { oldStart: 108,
       oldLines: 3,
       newStart: 113,
       newLines: 74,
       lines: [Object] },
     { oldStart: 30,
       oldLines: 3,
       newStart: 30,
       newLines: 7,
       lines: [Object] },
     { oldStart: 184,
       oldLines: 6,
       newStart: 184,
       newLines: 7,
       lines: [Object] },
     { oldStart: 200,
       oldLines: 6,
       newStart: 201,
       newLines: 7,
       lines: [Object] },
     { oldStart: 215,
       oldLines: 6,
       newStart: 217,
       newLines: 7,
       lines: [Object] },
     { oldStart: 28,
       oldLines: 6,
       newStart: 28,
       newLines: 8,
       lines: [Object] },
     { oldStart: 47,
       oldLines: 28,
       newStart: 47,
       newLines: 13,
       lines: [Object] },
     { oldStart: 26736,
       oldLines: 6,
       newStart: 26736,
       newLines: 9,
       lines: [Object] },
     { oldStart: 26769,
       oldLines: 6,
       newStart: 26772,
       newLines: 7,
       lines: [Object] },
     { oldStart: 26851,
       oldLines: 6,
       newStart: 26855,
       newLines: 9,
       lines: [Object] },
     { oldStart: 4719,
       oldLines: 6,
       newStart: 4719,
       newLines: 9,
       lines: [Object] },
     { oldStart: 4752,
       oldLines: 6,
       newStart: 4755,
       newLines: 7,
       lines: [Object] },
     { oldStart: 4817,
       oldLines: 6,
       newStart: 4821,
       newLines: 9,
       lines: [Object] },
     { oldStart: 19,
       oldLines: 7,
       newStart: 19,
       newLines: 8,
       lines: [Object] },
     { oldStart: 4,
       oldLines: 7,
       newStart: 4,
       newLines: 7,
       lines: [Object] },
     { oldStart: 125,
       oldLines: 6,
       newStart: 125,
       newLines: 7,
       lines: [Object] },
     { oldStart: 251,
       oldLines: 17,
       newStart: 252,
       newLines: 13,
       lines: [Object] },
     { oldStart: 295,
       oldLines: 7,
       newStart: 292,
       newLines: 7,
       lines: [Object] },
     { oldStart: 304,
       oldLines: 7,
       newStart: 301,
       newLines: 7,
       lines: [Object] },
     { oldStart: 361,
       oldLines: 7,
       newStart: 358,
       newLines: 6,
       lines: [Object] },
     { oldStart: 370,
       oldLines: 10,
       newStart: 366,
       newLines: 8,
       lines: [Object] },
     { oldStart: 407,
       oldLines: 7,
       newStart: 401,
       newLines: 7,
       lines: [Object] },
     { oldStart: 415,
       oldLines: 11,
       newStart: 409,
       newLines: 10,
       lines: [Object] },
     { oldStart: 820,
       oldLines: 10,
       newStart: 813,
       newLines: 6,
       lines: [Object] },
     { oldStart: 1132,
       oldLines: 8,
       newStart: 1121,
       newLines: 13,
       lines: [Object] },
     { oldStart: 1346,
       oldLines: 6,
       newStart: 1340,
       newLines: 7,
       lines: [Object] },
     { oldStart: 21,
       oldLines: 3,
       newStart: 21,
       newLines: 4,
       lines: [Object] },
     { oldStart: 30,
       oldLines: 3,
       newStart: 30,
       newLines: 7,
       lines: [Object] },
     { oldStart: 25,
       oldLines: 16,
       newStart: 25,
       newLines: 19,
       lines: [Object] },
     { oldStart: 101,
       oldLines: 5,
       newStart: 104,
       newLines: 6,
       lines: [Object] },
     { oldStart: 64,
       oldLines: 6,
       newStart: 64,
       newLines: 23,
       lines: [Object] },
     { oldStart: 40,
       oldLines: 7,
       newStart: 40,
       newLines: 11,
       lines: [Object] },
     { oldStart: 18,
       oldLines: 3,
       newStart: 18,
       newLines: 10,
       lines: [Object] },
     { oldStart: 2112,
       oldLines: 6,
       newStart: 2112,
       newLines: 10,
       lines: [Object] },
     { oldStart: 364,
       oldLines: 17,
       newStart: 364,
       newLines: 21,
       lines: [Object] },
     { oldStart: 18,
       oldLines: 3,
       newStart: 18,
       newLines: 4,
       lines: [Object] },
     { oldStart: 551,
       oldLines: 6,
       newStart: 551,
       newLines: 9,
       lines: [Object] },
     { oldStart: 611,
       oldLines: 7,
       newStart: 614,
       newLines: 8,
       lines: [Object] },
     { oldStart: 789,
       oldLines: 15,
       newStart: 793,
       newLines: 18,
       lines: [Object] },
     { oldStart: 923,
       oldLines: 6,
       newStart: 930,
       newLines: 7,
       lines: [Object] } ] }

As you can see, the file names on the patch are not added anywhere, and in fact the whole patch is considered to be for only one (anonimous) index. I think the problem is related to be using Index: as identifier of each one of the files, while each file is being separated with a diff line:

# HG changeset patch
# Parent f50bb54f331f73405131a30b4f353cfda1c70304
Use the generic implementation of libstdc++ primitives when we're on musl, not the glibc one.

diff -r f50bb54f331f libstdc++-v3/configure.host
--- a/libstdc++-v3/configure.host   Fri Mar 29 16:38:52 2013 -0400
+++ b/libstdc++-v3/configure.host   Fri Mar 29 16:41:10 2013 -0400
@@ -264,6 +264,13 @@
     os_include_dir="os/bsd/freebsd"
     ;;
   gnu* | linux* | kfreebsd*-gnu | knetbsd*-gnu)
+    # check for musl by target
+    case "${host_os}" in
+      *-musl*)
+        os_include_dir="os/generic"
+        ;;
+      *)
+
     if [ "$uclibc" = "yes" ]; then
       os_include_dir="os/uclibc"
     elif [ "$bionic" = "yes" ]; then
@@ -272,6 +279,9 @@
       os_include_dir="os/gnu-linux"
     fi
     ;;
+
+    esac
+    ;;
   hpux*)
     os_include_dir="os/hpux"
     ;;

I believe it would be as simple as allowing to use not only Index: as identifier but also diff too.

Also, I think that instead of ignoring the headers when no index is found, it would be good to add them although they are partial, or if it's done this way to consume the patch lines, maybe the var i would be increased directly without calling parseFileHeader() with an empty object...

@piranna
Copy link
Contributor Author

piranna commented Oct 13, 2015

Also, since applyPatches() works automatically and only need to ask for the content of the file, maybe the callbacks loadFile and patched would give the path instead of the full index object, since it will not be possible to use it in any case. And finally, in the same way, it would be good that complete callback give not only an error but also a mapping path-content, making the patched callback optional allowing a better flow control.

@piranna
Copy link
Contributor Author

piranna commented Oct 15, 2015

I've done a pull-request that seems to fix this, just by adding diff -r to the regular expressions:

{ index: 'libstdc++-v3/configure.host',
  oldFileName: 'a/libstdc++-v3/configure.host',
  oldHeader: 'Fri Mar 29 16:38:52 2013 -0400',
  newFileName: 'b/libstdc++-v3/configure.host',
  newHeader: 'Fri Mar 29 16:41:10 2013 -0400',
  hunks: 
   [ { oldStart: 264,
       oldLines: 6,
       newStart: 264,
       newLines: 13,
       lines: [Object] },
     { oldStart: 272,
       oldLines: 6,
       newStart: 279,
       newLines: 9,
       lines: [Object] } ] }

Much better, isn't it? :-D Only thing are the oldFilename and newFilename letters at beginning (a and b) that don't know if they should be skipped. They don't bother me so I've leave them as is. What do you think?

@kpdecker
Copy link
Owner

Closing in favor of #83

@piranna
Copy link
Contributor Author

piranna commented Oct 16, 2015

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants