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

dojo: trying to bind face sur results causes crash #5597

Open
ynx0 opened this issue Feb 12, 2022 · 8 comments
Open

dojo: trying to bind face sur results causes crash #5597

ynx0 opened this issue Feb 12, 2022 · 8 comments

Comments

@ynx0
Copy link
Contributor

ynx0 commented Feb 12, 2022

Describe the bug
When trying to set the face sur to any value, say 1, dojo crashes and unlinks itself.

To Reproduce
Steps to reproduce the behaviour:

  1. Type =sur 1 at the dojo prompt

Expected behaviour
Binding the face should work as any other. For instance, =mar 1 works fine.

System (please supply the following information, if relevant):

  • OS: NixOS
  • Vere 1.8 and Urbit urbit-v1.9-rc2
    -%base: ~ (this is a fakezod so...)

Additional context

Full stack trace:

drum: link [~nut %dojo]
[%dojo-peer-replaced ~.drum_~nut]
[unlinked from [p=~nut q=%dojo]]
/sys/vane/gall/hoon:<[1.372 9].[1.372 37]>
/app/dojo/hoon:<[1.585 3].[1.625 20]>
/app/dojo/hoon:<[1.586 3].[1.625 20]>
/app/dojo/hoon:<[1.587 5].[1.623 7]>
/app/dojo/hoon:<[1.588 5].[1.623 7]>
/app/dojo/hoon:<[1.591 7].[1.592 70]>
/app/dojo/hoon:<[1.592 7].[1.592 70]>
/app/dojo/hoon:<[1.592 15].[1.592 70]>
/app/dojo/hoon:<[1.490 5].[1.498 7]>
/app/dojo/hoon:<[1.491 5].[1.498 7]>
/app/dojo/hoon:<[1.493 5].[1.498 7]>
/app/dojo/hoon:<[1.495 13].[1.495 37]>
/app/dojo/hoon:<[1.268 5].[1.289 7]>
/app/dojo/hoon:<[1.269 5].[1.289 7]>
/app/dojo/hoon:<[1.276 5].[1.289 7]>
/app/dojo/hoon:<[1.277 5].[1.289 7]>
/app/dojo/hoon:<[1.280 7].[1.288 9]>
/app/dojo/hoon:<[1.282 7].[1.288 9]>
/app/dojo/hoon:<[1.283 7].[1.288 9]>
/app/dojo/hoon:<[1.284 7].[1.288 9]>
/app/dojo/hoon:<[1.285 7].[1.288 9]>
/app/dojo/hoon:<[1.286 13].[1.286 39]>
/app/dojo/hoon:<[1.262 5].[1.264 66]>
/app/dojo/hoon:<[1.263 5].[1.264 66]>
/app/dojo/hoon:<[1.264 5].[1.264 66]>
/app/dojo/hoon:<[1.264 13].[1.264 66]>
/app/dojo/hoon:<[985 7].[991 46]>
/app/dojo/hoon:<[986 7].[991 46]>
/app/dojo/hoon:<[987 7].[991 46]>
/app/dojo/hoon:<[989 7].[991 46]>
/app/dojo/hoon:<[991 7].[991 46]>
/app/dojo/hoon:<[895 7].[939 9]>
/app/dojo/hoon:<[896 7].[939 9]>
/app/dojo/hoon:<[897 7].[939 9]>
/app/dojo/hoon:<[898 7].[939 9]>
/app/dojo/hoon:<[901 16].[901 31]>
/app/dojo/hoon:<[961 7].[964 22]>
/app/dojo/hoon:<[962 7].[964 22]>
/app/dojo/hoon:<[964 7].[964 22]>
/app/dojo/hoon:<[488 7].[490 67]>
/app/dojo/hoon:<[489 7].[490 67]>
/app/dojo/hoon:<[490 7].[490 67]>
/app/dojo/hoon:<[985 7].[991 46]>
/app/dojo/hoon:<[986 7].[991 46]>
/app/dojo/hoon:<[987 7].[991 46]>
/app/dojo/hoon:<[988 9].[988 16]>
/app/dojo/hoon:<[529 7].[627 9]>
/app/dojo/hoon:<[532 7].[627 9]>
/app/dojo/hoon:<[534 7].[627 9]>
/app/dojo/hoon:<[543 7].[627 9]>
/app/dojo/hoon:<[544 7].[627 9]>
/app/dojo/hoon:<[546 9].[574 11]>
/app/dojo/hoon:<[547 9].[574 11]>
[%bad-set %sur #t/@ud]
/app/dojo/hoon:<[548 9].[574 11]>
/app/dojo/hoon:<[549 9].[574 11]>
/app/dojo/hoon:<[560 11].[563 13]>
/app/dojo/hoon:<[562 13].[562 71]>
/app/dojo/hoon:<[523 19].[526 18]>
/app/dojo/hoon:<[524 7].[526 18]>
[#t/it([face=u(@tas) file-path=@tas]) #t/@ud]
/app/dojo/hoon:<[525 7].[526 18]>
/app/dojo/hoon:<[525 11].[525 35]>
-need.?(%~ [i=[face=u(@tas) file-path=@tas] t=it([face=u(@tas) file-path=@tas])])
-have.@ud
nest-fail
[%drum-coup-fail ~nut p=~nut q=%dojo]
[linked to [p=~nut q=%dojo]]

Notify maintainers
best guesses:
@philipcmonk
@Fang-

@ynx0 ynx0 added the bug label Feb 12, 2022
@Fang-
Copy link
Member

Fang- commented Feb 12, 2022

Likely the same for lib. I think these are a remnant from back when dojo supported /- something as the equivalent to modern-day =something -build-file %/sur/something/hoon. Dojo still supports them syntax-wise, but the logic appears stubbed out:

%sur
%_ .
sur
((dy-cast (list cable:clay) !>(*(list cable:clay))) q.cay)
==

We should either revive this properly (we can scry built files now, right?) or axe the functionality (and its accompanying restrictions on the sur and lib names) entirely.

@Fang- Fang- added the dojo label Feb 12, 2022
@ohAitch
Copy link
Contributor

ohAitch commented Feb 13, 2022

Noting bias from being the one who implemented them in the first place - I do think "use libraries in the repl" is a generally useful capability. I think it mainly came up for

  • testing something you're in the process of writing
  • pasting and modifying examples in comments to better understand someone else's code
  • constructing one-off pokes without having to set up generator boilerplate

@ynx0
Copy link
Contributor Author

ynx0 commented Feb 13, 2022 via email

@ohAitch
Copy link
Contributor

ohAitch commented Feb 13, 2022

When you're actively modifying the library, it's significantly worse to have to rerun that every other line yeah
and then there's more minor things of, doesn't support /+ *graph-store, four times longer to type(assuming you say =graph-store not =lb so code you copy out of generators apps etc can run), and the above compounding when there's two libs and three surs involved

@ohAitch
Copy link
Contributor

ohAitch commented Feb 13, 2022

Like, imagine you have a sur/frob and lib/frob, the test case

/-  frob
/+  *frob
(populate-frob *frob 'zig' 'bam' ~)

and +=frob is a $% which you've just changed one of the %tags on - in the type, and correspondingly, usage of the type

In old dojo, you would rerun the test case with up-enter
With explicit -build-file, you rerun the test case with up-up-up-enter-up-up-up-enter-up-up-up-enter
Unless you've modified the test case a time or two, in which case good luck lol, you probably mouse over to a separate editor tab you keep for this purpose, copy the entire import lines, paste them into urbit, and wait half a minute for the dojo parser to churn through the input

Eventually you notice this is annoying, look up some app boilerplate, and write a testing app that ~&s the test case on reload instead. (Or maybe :test has rerun-on-modification functionality nowadays, and you write it as a test case?) This works well enough, but now you've lost the repl, you can no longer easily pass other data in or write the result to a file or w/e.
Also instead of having a line of code and a line of result right below it, you have a line of "this testing file changed" and a result below that, with a separate editor pane next to / above your terminal composed mostly of boilerplate but ending in the line you care about, but that's arguably more minor.

@ynx0
Copy link
Contributor Author

ynx0 commented Feb 13, 2022 via email

@ohAitch
Copy link
Contributor

ohAitch commented Feb 13, 2022

Yep, the need is not obviated which is not to say necessarily high-priority :P

@ohAitch
Copy link
Contributor

ohAitch commented Feb 13, 2022

Also I think even if it were revived, "assign an atom to sur" should still produce an error, but that error should not unlink dojo. (I'm honestly not sure what architectural feature is causing it to be possible for a poke crash to unlink dojo? This is definitely not the first bug in the genre)

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

No branches or pull requests

3 participants