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

Versioned webpage using commit directories #101

Merged
merged 17 commits into from
Feb 16, 2018
Merged

Conversation

dhimmel
Copy link
Member

@dhimmel dhimmel commented Jan 16, 2018

Work in progress

Closes #96

@dhimmel dhimmel force-pushed the issue-96 branch 2 times, most recently from a883441 to b18f8b4 Compare January 16, 2018 22:54
Not tested on case where pathspec v exists
Requires manubot to set variables ci_source.repo_owner and
ci_source.repo_name. Hence, upgrade to
manubot/manubot@2745659
@dhimmel
Copy link
Member Author

dhimmel commented Feb 3, 2018

Testing this in a manubot instance at https://github.com/dhimmel/code-96. Will cherry-pick additional commits from that repo back to this PR, when things are all working.

Seems to be mostly working. Remaining issues:

  • ghp-import does not follow symlinks, which we require for the site to work as intended Option to preserve symlinks c-w/ghp-import#70
  • version directories are missing an index.html file. This should probably be a symlink or renaming of manuscript.html. Alternatively, we could add a README.
  • SETUP.md: do we want to purge versions pre-fork?
  • Does webpage.py belong here or as part of the manubot python package?

@dhimmel
Copy link
Member Author

dhimmel commented Feb 12, 2018

version directories are missing an index.html file. This should probably be a symlink or renaming of manuscript.html

completed

SETUP.md: do we want to purge versions pre-fork?

Yes. completed

ghp-import does not follow symlinks, which we require for the site to work as intended

This PR now works (see https://github.com/dhimmel/code-96) even though ghp-import doesn't preserve symlinks. If ghp-import implements an option to preserve symlinks, the diffs on the webpage branch would become more simple.

@agitter ready for review

@agitter
Copy link
Member

agitter commented Feb 12, 2018

ready for review

Okay. I'll likely need a few days to get to this.

@dhimmel
Copy link
Member Author

dhimmel commented Feb 13, 2018

Here's an example commit on what upgrading OTS stamps now looks like dhimmel/code-96@8dae193. Requires a bitcoin full node.

@dhimmel
Copy link
Member Author

dhimmel commented Feb 13, 2018

I created a script https://gist.github.com/dhimmel/5cf98acc58f60ede9504422e7a0a9f41 that will take a legacy gh-pages and create version directories for past commits. This is not required, but allows you to pull out historical versions into their own directories.

Copy link
Member

@agitter agitter left a comment

Choose a reason for hiding this comment

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

Looks great overall. I only have questions and some minor comments.

The [`v`](v) directory contains directories for each manuscript version.
In general, a version is identified by the commit hash of the source content that created it.
The `*.ots` files in version directories are OpenTimestamps which can be used to verify manuscript existence at or before a given time.
[OpenTimestamps](opentimestamps.org) uses the Bitcoin blockchain to attest to file hash existence.
Copy link
Member

Choose a reason for hiding this comment

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

This is being treated as a relative link.

--checkout=gh-pages \
--version=$TRAVIS_COMMIT

# Generate OpenTimestamps
Copy link
Member

Choose a reason for hiding this comment

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

I want to make sure I understand the timestamping strategy. We still timestamp in the same way without --wait because we want to CI deploy script to terminate quickly. However, we now have the ability to manually update all of the OTS Timestamps whenever we want as you did in dhimmel/code-96@8dae193 without rewriting history. After the upgrade, those timestamps are no longer dependent on the calendar service.

Is that correct? Do we need to document the upgrade instructions somewhere?

Copy link
Member Author

@dhimmel dhimmel Feb 16, 2018

Choose a reason for hiding this comment

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

Yes your understanding is correct. --wait is not feasible for CI builds as it could take days IIRC. Will document.

README.md Outdated
@@ -53,6 +53,9 @@ sh build/build.sh
# when a change is detected.
sh build/autobuild.sh

# Configure the webpage directory
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be an optional step for a local build. If sh build/build.sh is still the minimal command needed to build the PDF and HTML manuscripts, we may want to indicate that more explicitly in the script comments or preceding text.

+ `*.ots` files are OpenTimestamps which can be used to verify manuscript existence at or before a given time.
[OpenTimestamps](opentimestamps.org) uses the Bitcoin blockchain to attest to file hash existence.

The [`v`](v) directory contains directories for each manuscript version.
Copy link
Member

Choose a reason for hiding this comment

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

Do we want v to be a link if it is ignored on master?

@dhimmel dhimmel merged commit 29cd993 into manubot:master Feb 16, 2018
@dhimmel dhimmel deleted the issue-96 branch February 16, 2018 18:50
dhimmel added a commit that referenced this pull request Feb 16, 2018
This build is based on
29cd993.

This commit was created by the following Travis CI build and job:
https://travis-ci.org/greenelab/manubot-rootstock/builds/342477457
https://travis-ci.org/greenelab/manubot-rootstock/jobs/342477458

[ci skip]

The full commit message that triggered this build is copied below:

Versioned webpage using commit directories (#101)

Closes #96
dhimmel added a commit that referenced this pull request Feb 16, 2018
This build is based on
29cd993.

This commit was created by the following Travis CI build and job:
https://travis-ci.org/greenelab/manubot-rootstock/builds/342477457
https://travis-ci.org/greenelab/manubot-rootstock/jobs/342477458

[ci skip]

The full commit message that triggered this build is copied below:

Versioned webpage using commit directories (#101)

Closes #96
@dhimmel
Copy link
Member Author

dhimmel commented Feb 16, 2018

Note the ots upgrade command added to the documentation in this PR may fail due to opentimestamps/opentimestamps-client#71.

@slochower
Copy link
Collaborator

I think there may be an issue with this commit. I'll be brief because I have to run, but can fill in details later if the fix is not obvious to you.

I'm running locally with sh build/autobuild.sh and viewing the local HTML output with python -m http.server in webpage/. If I update my content directory, I see the autobuild run correctly and can verify the word count increasing. However, the webpage I see in my browser is not updated. Upon looking closer, it does not seem that the html files in webpage/v are being updated correctly at all. For example, I just added the text "(try 2)" to one of my Markdown files, hit save, and saw the build complete. But if I look for that phrase in my webpage/v directory using find . -name "*.html" -exec grep -Hni "(try 2)" {} \;, I get nothing. Since PDF is built from the webpage, I don't see my changes there, either. I can't be sure which commit caused this, but I think it may be related to this feature.

I can dig deeper later, if you are unable to confirm this behavior. FWIW, here is the output of autobuild.sh:

Retrieving and processing reference metadata
## INFO
Manuscript content parts:
00.front-matter
01.recent-observations
99.back-matter
## INFO
0 unique citations strings extracted from text
0 unique standard citations    
## INFO
requests-cache starting with 0 cached responses
## INFO
requests-cache finished with 0 cached responses
## INFO
Using UTC timezone.
Dating manuscript with the current datetime: 2018-02-17T01:15:54.189811+00:00
## INFO
Reading user-provided templating variables complete:
{}
## INFO
Generated manscript stats:
{
  "reference_counts": {
    "total": 0
  },
  "word_count": 1361
}
Exporting HTML manuscript
Exporting PDF manuscript
WARNING: Ignored `-ms-text-size-adjust: 100%` at 78:5, unknown property.
WARNING: Ignored `-webkit-text-size-adjust: 100%` at 79:5, unknown property.
WARNING: Ignored `-moz-box-sizing: content-box` at 204:5, unknown property.
WARNING: Ignored `-webkit-appearance: button` at 379:5, unknown property.
WARNING: Ignored `cursor: pointer` at 380:5, the property does not apply for the print media.
WARNING: Ignored `cursor: default` at 389:5, the property does not apply for the print media.
WARNING: Ignored `-webkit-appearance: textfield` at 410:5, unknown property.
WARNING: Ignored `-moz-box-sizing: content-box` at 411:5, unknown property.
WARNING: Ignored `-webkit-box-sizing: content-box` at 412:5, unknown property.
WARNING: Ignored `-webkit-appearance: none` at 423:5, unknown property.
WARNING: Invalid or unsupported selector 'button::-moz-focus-inner,
input::-moz-focus-inner ', Unknown pseudo-element: -moz-focus-inner
WARNING: Invalid or unsupported selector '*:not("#mkdbuttons") ', (<FunctionBlock not( … )>, ':not() only accepts a simple selector')
WARNING: Ignored `-webkit-font-smoothing: subpixel-antialiased` at 486:5, unknown property.
WARNING: Ignored `-moz-border-radius: 3px` at 491:5, unknown property.
WARNING: Ignored `-webkit-border-radius: 3px
` at 492:5, unknown property.
WARNING: Ignored `-webkit-font-smoothing: subpixel-antialiased` at 528:5, unknown property.
WARNING: Ignored `cursor: text
` at 529:5, the property does not apply for the print media.
WARNING: Ignored `word-break: break-all` at 733:5, unknown property.
WARNING: Ignored `word-break: break-word` at 734:5, unknown property.
WARNING: Ignored `-webkit-hyphens: auto` at 735:5, unknown property.
WARNING: Ignored `-moz-hyphens: auto` at 736:5, unknown property.
Build complete

@dhimmel
Copy link
Member Author

dhimmel commented Feb 17, 2018

Thanks for the report @slochower.

You need to run the following to touch the webpage directory:

python build/webpage.py

Previously, index.html was a symlink to the output directory. But now index.html requires webpage.py to be updated.

Sorry to break your autobuild workflow. How useful have you found the autobuild script? Since webpage.py accepts arguments, it's not entirely clear to me how we should fix it. Probably just add python build/webpage.py with the default arguments to autobuild.sh.

https://github.com/greenelab/manubot-rootstock/blob/29cd993b0b6b9c41273e306f4c17262ffdbdf546/build/webpage.py#L13-L22

@slochower
Copy link
Collaborator

Thanks for the quick reply. Interesting change. I agree that we should probably put python build/webpage.py in autobuild.sh and I think the default arguments are fine (at least for me) because when I run autobuild.sh it's usually because I'm trying to do look at something quickly locally.

I mostly use this local workflow when I want to see my rendered document (for example, if I have a bunch of equations and figures). This comes in handy for more informal cases than writing a collaborative manuscript. Sometimes I want to write a complicated email with figures and syntax highlighting and so I start one of these Markdown documents -- but it doesn't make sense to use an existing manubot-rootstock directory for a manuscript and I don't necessarily want to setup a new GitHub repository just for a single document, so I lazily clone manubot-rootstock and just run it in local mode to generate an HTML or PDF document that I can send.

dhimmel added a commit to dhimmel/manubot-rootstock that referenced this pull request Feb 17, 2018
Most autobuild users will want to automatically update the webpage. See discussion at
manubot#101 (comment)

Update README to better document autobuild.
dhimmel added a commit that referenced this pull request Feb 17, 2018
Most autobuild users will want to automatically update the webpage. See discussion at
#101 (comment)

Update README to better document autobuild.
dhimmel added a commit that referenced this pull request Feb 17, 2018
This build is based on
b7a9de8.

This commit was created by the following Travis CI build and job:
https://travis-ci.org/greenelab/manubot-rootstock/builds/342797312
https://travis-ci.org/greenelab/manubot-rootstock/jobs/342797313

[ci skip]

The full commit message that triggered this build is copied below:

autobuild.sh: include webpage.py execution (#113)

Most autobuild users will want to automatically update the webpage. See discussion at
#101 (comment)

Update README to better document autobuild.
dhimmel added a commit that referenced this pull request Feb 17, 2018
This build is based on
b7a9de8.

This commit was created by the following Travis CI build and job:
https://travis-ci.org/greenelab/manubot-rootstock/builds/342797312
https://travis-ci.org/greenelab/manubot-rootstock/jobs/342797313

[ci skip]

The full commit message that triggered this build is copied below:

autobuild.sh: include webpage.py execution (#113)

Most autobuild users will want to automatically update the webpage. See discussion at
#101 (comment)

Update README to better document autobuild.
adebali pushed a commit to CompGenomeLab/lemur-manuscript-archive that referenced this pull request Mar 4, 2020
Most autobuild users will want to automatically update the webpage. See discussion at
manubot/rootstock#101 (comment)

Update README to better document autobuild.
ploegieku added a commit to ploegieku/2023-functional-homology-paper that referenced this pull request Aug 6, 2024
Most autobuild users will want to automatically update the webpage. See discussion at
manubot/rootstock#101 (comment)

Update README to better document autobuild.
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