cryogen-project / cryogen

A simple static site generator written in Clojure
http://cryogenweb.org/
Eclipse Public License 1.0
1.09k stars 96 forks source link

Support for incremental compilation #220

Closed holyjak closed 3 years ago

holyjak commented 3 years ago

Depends on cryogen-project/cryogen-core#149

bombaywalla commented 3 years ago

I merged cryogen-core PR#149, which this PR depends on.

From my reading of the code, it seems this PR is not absolutely required. Because you maintained backward compatibility with existing code when you did PR#149. Is that correct?

It is also my understanding that with this PR (or even without), the old behavior of doing a full compilation still functions as it used to (meaning that it does not do incremental compilation by default). Is that correct?

I see that this PR updates the code to use the newer start watcher-for-changes! and that seems like a good thing. So, I'd be okay merging this PR if my understandings, above, are correct.

holyjak commented 3 years ago

Actually we would need to release core and bump the version number here first to have the new function available so we should wait with the merge.

On Mon, 30 Nov 2020, 19:47 Dorab Patel, notifications@github.com wrote:

I merged cryogen-core PR#149, which this PR depends on.

From my reading of the code, it seems this PR is not absolutely required. Because you maintained backward compatibility with existing code when you did PR#149. Is that correct?

It is also my understanding that with this PR (or even without), the old behavior of doing a full compilation still functions as it used to (meaning that it does not do incremental compilation by default). Is that correct?

I see that this PR updates the code to use the newer start watcher-for-changes! and that seems like a good thing. So, I'd be okay merging this PR if my understandings, above, are correct.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cryogen-project/cryogen/pull/220#issuecomment-735971597, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEYSPU2DCSRONBMBVRG6H3SSPSEVANCNFSM4TBRPHVQ .

bombaywalla commented 3 years ago

Right. I am working on making a release to cryogen-core. I thought I'd take the opportunity to add some of the support files (like the ones you pointed me at).

holyjak commented 3 years ago

Updated to be opt-in, adding lein serve[-fast] and clojure -X:serve-fast for running with the fast compilation turned on.

bombaywalla commented 3 years ago

Please update this PR as per changes in upstream. the cryogen-core versions should be 0.4.0 in the generated deps.edn and generated project.clj.

Also, it seems you changed one of the sample files, but did not update the line in cryogen.clj that compiles the file. The following diff fixes that issue.

diff --git a/src/leiningen/new/cryogen.clj b/src/leiningen/new/cryogen.clj
index 74bbefa..985cc18 100644
--- a/src/leiningen/new/cryogen.clj
+++ b/src/leiningen/new/cryogen.clj
@@ -120,7 +120,7 @@
              ["content/md/pages/another-page.md" (render "md/pages/another-page.md")]
              ["content/md/posts/2014-03-10-first-post.md" (render "md/posts/2014-03-10-first-post.md")]
              ["content/md/posts/2014-11-04-second-post.md" (render "md/posts/2014-11-04-second-post.md")]
-             ["content/md/posts/2016-01-07-docs.md" (render "md/posts/2016-01-07-docs.md")]
+             ["content/md/posts/2020-12-03-docs.md" (render "md/posts/2020-12-03-docs.md")]
              ;;Asciidoc templates
              ["content/asc/pages/adoc-page.asc" (render "asc/pages/adoc-page.asc")]
              ["content/asc/posts/2014-10-10-adoc-post.asc" (render "asc/posts/2014-10-10-adoc-post.asc")]
bombaywalla commented 3 years ago

I tried testing this PR with lein and it seems to work. I still have to figure out a way to test it with the clojure CLI.

One other thing that I came across was that in fast compilation mode, sometimes (and I have not been able to reproduce the situation reliably) the system recompiles the changed file every second change that is saved to disk. I presume that the problem (if any) is in cryogen-core.

holyjak commented 3 years ago

Thanks a lot for the catch! Fixed everything now. How did you discover the problem? Is it possible to run lein new with a local repo as the template??

Clojure new uses the same process as lein new so I assume if the one works, the other will two.

the system recompiles the changed file every second change

I have not seen that. It could be something about your OS and the change detection lib that we use. Nothing we can do about it, I am afraid.

bombaywalla commented 3 years ago

How did you discover the problem? Is it possible to run lein new with a local repo as the template??

cd dir-containing-repo-with-220-applied
lein new cryogen myblog
bombaywalla commented 3 years ago

I was not able to get this to work as stated (complains about not being able to create the files) clojure -X:new create :template cryogen :name me/my-blog. However, I was able to get this to work clojure -X:new create :template cryogen :name me.my-blog.

Would you confirm? Or tell me what I am doing wrong?

I will make another attempt to track down the circumstances under which I'm getting the "recompile every second change" issue. I'm on MacOS in case that matters. I can also try it on Linux.

holyjak commented 3 years ago

According to https://github.com/seancorfield/Clj-new it should be / not dot. I am also on osx.

-------- Original Message -------- On 7 Dec 2020, 18:23, Dorab Patel < notifications@github.com> wrote:

I was not able to get this to work as stated (complains about not being able to create the files) clojure -X:new create :template cryogen :name me/my-blog. However, I was able to get this to work clojure -X:new create :template cryogen :name me.my-blog.

Would you confirm? Or tell me what I am doing wrong?

I will make another attempt to track down the circumstances under which I'm getting the "recompile every second change" issue. I'm on MacOS in case that matters. I can also try it on Linux.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.AAEYSPTROIFTUSXDCWSCTNDSTUFSTA5CNFSM4TBRPHV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFQOHAVY.gif

bombaywalla commented 3 years ago
Dorabs-iMac:prs dorab$ git clone https://github.com/holyjak/cryogen
Cloning into 'cryogen'...
remote: Enumerating objects: 81, done.
remote: Counting objects: 100% (81/81), done.
remote: Compressing objects: 100% (47/47), done.
remote: Total 2680 (delta 29), reused 54 (delta 17), pack-reused 2599
Receiving objects: 100% (2680/2680), 914.09 KiB | 1.77 MiB/s, done.
Resolving deltas: 100% (1183/1183), done.
Dorabs-iMac:prs dorab$ cd cryogen/
Dorabs-iMac:cryogen dorab$ ls
LICENSE     README.md   cryogen.png project.clj src
Dorabs-iMac:cryogen dorab$ git checkout --track origin/feat/fast-compile
Branch 'feat/fast-compile' set up to track remote branch 'feat/fast-compile' from 'origin'.
Switched to a new branch 'feat/fast-compile'
Dorabs-iMac:cryogen dorab$ clojure -X:new create :template cryogen :name me/my-blog
Downloading: cryogen/lein-template/maven-metadata.xml from clojars
Generating fresh 'lein new' Cryogen project.
Could not create directory /Users/dorab/Projects/prs/cryogen/me/my-blog. Maybe it already exists?  See also :force or --force
Dorabs-iMac:cryogen dorab$ clojure -X:new create :template cryogen :name me.my-blog
Generating fresh 'lein new' Cryogen project.
Dorabs-iMac:cryogen dorab$ 

I think this is happening because even though we are using clj-new, the template is a lein template that does not know how to create a project with a qualified name.

From clj-new's README...

As noted above, project-name should be a qualified symbol, such as mygithubusername/my-new-project, or a multi-segment symbol, such as my.cool.project. Some templates will not work with the former but it is recommended you try that format first.

bombaywalla commented 3 years ago

I can reliably reproduce the "second change" issue. It does not seem to always be the second change. In any case, it seems to be a problem with cryogen-core (or the watcher lib). Here is how I can reproduce the issue. Would you please try it out and see if you can get it to fail as well.

Dorabs-iMac:prs dorab$ git clone https://github.com/holyjak/cryogen
Cloning into 'cryogen'...
remote: Enumerating objects: 81, done.
remote: Counting objects: 100% (81/81), done.
remote: Compressing objects: 100% (47/47), done.
remote: Total 2680 (delta 29), reused 54 (delta 17), pack-reused 2599
Receiving objects: 100% (2680/2680), 914.09 KiB | 1.82 MiB/s, done.
Resolving deltas: 100% (1183/1183), done.
Dorabs-iMac:prs dorab$ cd cryogen
Dorabs-iMac:cryogen dorab$ git checkout --track origin/feat/fast-compile
Branch 'feat/fast-compile' set up to track remote branch 'feat/fast-compile' from 'origin'.
Switched to a new branch 'feat/fast-compile'
Dorabs-iMac:cryogen dorab$ lein new cryogen myblog
OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were deprecated in JDK 13 and will likely be removed in a future release.
Generating fresh 'lein new' Cryogen project.
Dorabs-iMac:cryogen dorab$ cd myblog
Dorabs-iMac:myblog dorab$ clojure -X:serve-fast
holyjak commented 3 years ago

Oh, that makes sense! I will adjust the docs... Thank you for tracking this down, great job!

-------- Original Message -------- On 7 Dec 2020, 18:37, Dorab Patel < notifications@github.com> wrote:

Dorabs-iMac:prs dorab$ git clone https://github.com/holyjak/cryogen
Cloning into 'cryogen'...
remote: Enumerating objects: 81, done.
remote: Counting objects: 100% (81/81), done.
remote: Compressing objects: 100% (47/47), done.
remote: Total 2680 (delta 29), reused 54 (delta 17), pack-reused 2599
Receiving objects: 100% (2680/2680), 914.09 KiB | 1.77 MiB/s, done.
Resolving deltas: 100% (1183/1183), done.
Dorabs-iMac:prs dorab$ cd cryogen/
Dorabs-iMac:cryogen dorab$ ls
LICENSE       README.md   cryogen.png project.clj src
Dorabs-iMac:cryogen dorab$ git checkout --track origin/feat/fast-compile
Branch 'feat/fast-compile' set up to track remote branch 'feat/fast-compile' from 'origin'.
Switched to a new branch 'feat/fast-compile'
Dorabs-iMac:cryogen dorab$ clojure -X:new create :template cryogen :name me/my-blog
Downloading: cryogen/lein-template/maven-metadata.xml from clojars
Generating fresh 'lein new' Cryogen project.
Could not create directory /Users/dorab/Projects/prs/cryogen/me/my-blog. Maybe it already exists?  See also :force or --force
Dorabs-iMac:cryogen dorab$ clojure -X:new create :template cryogen :name me.my-blog
Generating fresh 'lein new' Cryogen project.
Dorabs-iMac:cryogen dorab$

I think this is happening because even though we are using clj-new, the template is a lein template that does not know how to create a project with a qualified name.

From clj-new's README...

As noted above, project-name should be a qualified symbol, such as mygithubusername/my-new-project, or a multi-segment symbol, such as my.cool.project. Some templates will not work with the former but it is recommended you try that format first.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.AAEYSPURMQ5WE2AIZMMUBGDSTUHELA5CNFSM4TBRPHV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFQOI24Y.gif

bombaywalla commented 3 years ago

Great. After you've updated this PR with the clojure -X:new create :template cryogen :name me.my-blog change, I think this PR is ready to merge.

There is still the "second change" issue. But it seems that the problem is not in this repo.