pijul_org / pijul

#43 pijul record problem

Opened by lthms, on April 10, 2017
Closed
lthms commented on April 10, 2017

I have a code snippet that shows a strange issue in pijul record.\r + \r + I will have some trouble to give you a proper test scenario, but here is what I see\r + \r +

  1. When I first record, there is a diff pijul handles wrong: it does not see an empty line\r +
  2. If I record a second time, pijul actually see the empty line, but the different diff parts are strange somehow\r +
  3. After that, pijul internal representation of the file shows (with pijul status for instance) that it has mixed several lines.\r+ \r +

After the first record\r +

\r +

In file "/home/lethom/Repos/llkn/apps/core/lib/lkn/core.ex"\r                                                                     +
\r                                                                                                                                +
-     end\r                                                                                                                       +
-       {:via, Registry, {Lkn.Core.Registry, {:puppeteer, puppeteer_key}}}\r                                                      +
-     def puppeteer(puppeteer_key) do\r                                                                                           +
\r                                                                                                                                +
+ \r                                                                                                                              +
\r                                                                                                                                +
+     def puppeteer(puppeteer_key) do\r                                                                                           +
+       {:via, Registry, {Lkn.Core.Registry, {:puppeteer, puppeteer_key}}}\r                                                      +
+     end\r                                                                                                                       +
```\r                                                                                                                             +
\r                                                                                                                                +
## After the second record\r                                                                                                      +
\r                                                                                                                                +
```\r                                                                                                                             +
In file "/home/lethom/Repos/llkn/apps/core/lib/lkn/core.ex"\r                                                                     +
\r                                                                                                                                +
-     end\r                                                                                                                       +
-       {:via, Registry, {Lkn.Core.Registry, {:puppeteer, puppeteer_key}}}\r                                                      +
-     def puppeteer(puppeteer_key) do\r                                                                                           +
\r                                                                                                                                +
+     def puppeteer(puppeteer_key) do\r                                                                                           +
+       {:via, Registry, {Lkn.Core.Registry, {:puppeteer, puppeteer_key}}}\r                                                      +
+     end\r                                                                                                                       +
pmeunier commented on April 10, 2017

Can you create a repository or a branch on the nest, and push all your patches there?

lthms commented on April 10, 2017

I put everything there: https://nest.pijul.com/lthms/lkn/patches\r + \r + I’m sorry, there is I think some garbage… But for instance I only see one #12 patch in my pijul changes output.

lthms commented on April 10, 2017

Okay! I have cleaned everything. Now I can tell I have to record three times before pijul becomes happy with my patch.

pmeunier commented on April 10, 2017

I cloned your repository, unrecorded both patches, and cloned the resulting repository. The goal of doing that was to focus the debugging on applies only (unrecord is nontrivial and the implementation could have bugs).\r+ \r + There is a conflict in your file. Not sure how you got there, but the issue is that Pijul doesn't report conflicts properly. Our record/diff is aware of conflicts, but the UI is not there yet.

lthms commented on April 10, 2017

Not sure either :. I work a lot with partial records, maybe something lies here?\r + \r + Could you give me some additional info about the conflict? Is it in the current state of my repo? Why two records to solve it? Thanks!

lthms commented on April 10, 2017

Did some digging.\r + \r + The conflict is introduced by the first patch. I am almost sure this is a pijul bug, because here is how I did this record:\r+ \r +

  1. init a new repository\r +
  2. pull previous patch from the nest\r +
  3. copy my changes from a backup repo\r +
  4. pijul record -a
pmeunier commented on April 10, 2017

The conflict is in that same file. You can simply open the file and you will see it between >>>>>>>>>> and <<<<<<<<<<<<<. I'm trying to understand what type of conflict this is.

lthms commented on April 10, 2017

So if you want to reproduce, you can do that:\r + \r +

pijul clone https://nest.pijul.com/lthms/lkn\r               +
mkdir test-lkn\r                                             +
cd test-lkn\r                                                +
pijul init\r                                                 +
pijul pull ../lkn # say no to patch the three wrong patches\r+
cd ..\r                                                      +
find lkn/apps -name '*.ex' -exec cp -v {} test-{} \;\r       +
find lkn/apps -name '*.exs' -exec cp -v {} test-{} \;\r      +
cd test-lkn\r                                                +
pijul record -a\r                                            +
```\r                                                        +
\r                                                           +
Hope this help!
pmeunier commented on April 10, 2017

It turns out I can reproduce! I'm currently trying to display more info about the conflict, running for instance RUST_LOG="libpijul::graph=debug" pijul revert.\r + \r + It does show something strange. I believe it is related to a recent fix to a stack overflow in the algorithm computing strongly connected components (made it tail recursive, but it's not as easy to write).

lthms commented on April 10, 2017

Glad to here I could help!

pmeunier commented on April 10, 2017

Yes, thanks a lot for your time and patience.\r + \r + I just finished writing new debugging commands specifically for your case, and they do show what the problem is.\r + \r + So, one of the patches introduces a conflict, and there is nothing we can do to correct it now. I'll investigate what led to this situation.

lthms commented on April 10, 2017

> Yes, thanks a lot for your time and patience.\r + \r + My pleasure.\r + \r + > So, one of the patches introduces a conflict, and there is nothing we can do to correct it now. I'll investigate what led to this situation.\r + \r + If you can fix the issue before I have the time to work on lkn again, I will unrecord the guilty patch and just record it with the bug-free pijul. Otherwise, I will probably keep the two-patches fix I found, but rename them to be more explicit.

pmeunier commented on April 10, 2017

Wait, Pijul is not git: patches commute!.\r + \r + I know it's hard to believe, but there is no commit in Pijul!. As long as you keep working on independent changes, you can still unrecord old patches, even without branches. And even after that, just dependent patches need changes (maybe unrecord).

lthms commented on April 10, 2017

I know, but my future patches have a great chance to depend on this particular one unfortunately!

lthms commented on April 10, 2017

So, just to be sure: it is expected that I can no longer record the incriminated patch with the pijul I just built?

pmeunier commented on April 10, 2017

Not with the one you could build when you posted that comment, but with the current one, you can't record such a patch anymore (and pijul works "as git -p").

lthms commented on April 11, 2017

I’ve just tested your patches! (1) it works and (2) I really enjoy the new algorithm, it is more usable from my point of view.

lthms commented on April 11, 2017

I think we can conclude it is fixed.