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

publish: fix bug in publish comment mark #2904

Merged
merged 1 commit into from
May 16, 2020

Conversation

ixv
Copy link
Contributor

@ixv ixv commented May 16, 2020

Introduced by a stupid mistake I made in this PR: #2707 where I forgot that changes to a mark type definition need to include the old mark type. This should fix this issue: #2731

I've tested this fix by booting a ship from before #2707, making a notebook with comments, adding the code from #2707 and confirming the reproduction, then copying these changes in.

Apologies to everyone this issue has plagued.

@ixv ixv requested a review from philipcmonk May 16, 2020 15:07
Copy link
Contributor

@philipcmonk philipcmonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ixv ixv merged commit f18bd36 into release/next-userspace May 16, 2020
@philipcmonk
Copy link
Contributor

philipcmonk commented May 16, 2020

I used this to rescue a ship which had this bug. Since the bug breaks |mount, I ran this command to install the fix enough to mount:

*%/mar/publish/comment/hoon '/-  *publish\0a|_  com=?(comment-2 comment-3)\0a::\0a::\0a++  grow\0a  |%\0a  ++  mime\0a    :-  /text/x-publish-comments\0a    (as-octs:mimes:html (of-wain:format txt))\0a  ++  txt\0a    ^-  wain\0a    ?:  ?=(comment-2 com)\0a      :*  (cat 3 \'author: \' (scot %p author.com))\0a          (cat 3 \'date-created: \' (scot %da date-created.com))\0a          \'-----\'\0a          (to-wain:format content.com)\0a      ==\0a    ?>  ?=(comment-3 com)\0a    :*  (cat 3 \'author: \' (scot %p author.com))\0a        (cat 3 \'date-created: \' (scot %da date-created.com))\0a        \'-----\'\0a        (to-wain:format content.com)\0a    ==\0a  --\0a++  grab\0a  |%\0a  ++  mime\0a    |=  [mite:eyre p=octs:eyre]\0a    |^  (rash q.p both-parser)\0a    ++  key-val\0a      |*  [key=rule val=rule]\0a      ;~(sfix ;~(pfix key val) gaq)\0a    ++  old-parser\0a      ;~  plug\0a        (key-val (jest \'creator: ~\') fed:ag)\0a        (key-val (jest \'collection: \') sym)\0a        (key-val (jest \'post: \') sym)\0a        (key-val (jest \'date-created: ~\') (cook year when:so))\0a        (key-val (jest \'last-modified: ~\') (cook year when:so))\0a        ;~(pfix (jest (cat 3 \'-----\' 10)) (cook crip (star next)))\0a      ==\0a    ++  new-parser\0a      ;~  plug\0a        (key-val (jest \'author: ~\') fed:ag)\0a        (key-val (jest \'date-created: ~\') (cook year when:so))\0a        ;~(pfix (jest (cat 3 \'-----\' 10)) (cook crip (star next)))\0a      ==\0a    ++  both-parser\0a      ;~  pose\0a        %+  cook\0a          |=  [author=@ date-created=@da content=@t]\0a          ^-  comment\0a          [author date-created content %.n]\0a        new-parser\0a        %+  cook\0a          |=  [author=@ @ @ date-created=@da @ content=@t]\0a          ^-  comment\0a          [author date-created content %.n]\0a        old-parser\0a      ==\0a    --\0a  ++  noun  comment\0a  --\0a++  grad  %mime\0a--\0a'

Edit: @ixv notes that making this change without changing the app could break publish's read loop. Once the app has been committed (maybe by OTA), run :publish %reset-warp.

@ohAitch
Copy link
Contributor

ohAitch commented May 16, 2020 via email

@ixv
Copy link
Contributor Author

ixv commented May 16, 2020

oh lord 😔

@ohAitch
Copy link
Contributor

ohAitch commented May 16, 2020

I suppose alternately you can always *%/mar/publish/comment/hoon .^(@t %cx /~someone-else==/mar/publish/comment/hoon

@philipcmonk
Copy link
Contributor

Well, the Ford Fusion answer is "yes, a single bad mark should break |mount, but also it should kill the commit event so |mount will never actually be broken".

@ixv
Copy link
Contributor Author

ixv commented May 16, 2020

By the way, just changing the comment mark without the app changes may potentially break the read loop of publish. You should run :publish %reset-warp after committing the app.

@philipcmonk
Copy link
Contributor

Would it be best for me to amend the above command to commit both files? I guess the app then the mark?

@ixv
Copy link
Contributor Author

ixv commented May 16, 2020

Yeah that may be safest.

@philipcmonk
Copy link
Contributor

Actually the app is quite large, that may not be wise. Hmm...

@ixv
Copy link
Contributor Author

ixv commented May 16, 2020

I'll put the changes up on my ship and you can just reference them

@ohAitch
Copy link
Contributor

ohAitch commented May 16, 2020

That is certainly a coherent position, which would result in not being able to break |mount with a single bad mark!

@ixv
Copy link
Contributor Author

ixv commented May 17, 2020

Alright, the files are up on my ship

*%/app/publish/hoon .^(@t %cx /~novlud-padtyv/kids=/app/publish/hoon)
*%/mar/publish/comment/hoon .^(@t %cx /~novlud-padtyv/kids=/mar/publish/comment/hoon)

@philipcmonk
Copy link
Contributor

Have you tried this .^ from another ship that's not a moon of ~novlud or otherwise synced from it? Some related operations don't work, but that might. I would check but ~wicdev is in drydock at the moment.

@ixv
Copy link
Contributor Author

ixv commented May 17, 2020

Hm, you might be right, tried to run it on another ship and it's been amesing for an awfully long time.

Oh well, this is just a precaution, I'm confident that the %reset-warp thing will work. Or we can tell people to |merge from my %kids desk.

@ixv
Copy link
Contributor Author

ixv commented May 17, 2020

I ran it again with |ames-verb %rcv on, and it just hangs 2 fragments short of finishing.

@philipcmonk
Copy link
Contributor

I suspect it's compiling some new zuse. If that finishes it might succeed. I don't now that that won't be a problem in ford fusion, but the mechanics are all different, so it's not worth investigating here. Some problems are certainly fixed, like how on master you can't merge from a foreign desk into a new desk, it has to start from a local desk. On fusion this works fine.

@tacryt-socryp tacryt-socryp deleted the ixv/publish-comment-fix branch September 29, 2020 15:59
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

Successfully merging this pull request may close these issues.

3 participants