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

rewrites rules for dpl-docs file name style change #951

Merged
merged 1 commit into from
Apr 2, 2015

Conversation

MartinNowak
Copy link
Member

  • add RewriteMap that maps all current modules

- add RewriteMap that maps all current modules
@CyberShadow
Copy link
Member

Oh dear, I hope this doesn't impact Apache's performance too much.
Perhaps we should put this under /library/.htaccess instead.

Also, isn't this missing something? A RewriteMap declaration?

@MartinNowak
Copy link
Member Author

I generated the file by hacking ddox 0.10.2.

diff --git a/source/ddox/htmlgenerator.d b/source/ddox/htmlgenerator.d
index 0491f90..83ac21d 100644
--- a/source/ddox/htmlgenerator.d
+++ b/source/ddox/htmlgenerator.d
@@ -33,6 +33,8 @@ import vibe.templ.diet;

 void generateHtmlDocs(Path dst_path, Package root, GeneratorSettings settings = null)
 {
+   import std.stdio;
+   auto rewriteMap = File("rewrite_map.txt", "a");
    import std.algorithm : splitter;
    import vibe.web.common : adjustMethodStyle;

@@ -86,7 +88,7 @@ void generateHtmlDocs(Path dst_path, Package root, GeneratorSettings settings =
        return dst.data();
    }

-   void collectChildren(Entity parent, ref DocGroup[][string] pages)
+   void collectChildren(Entity parent, ref DocGroup[][string] pages, Path modpath)
    {
        Declaration[] members;
        if (auto mod = cast(Module)parent) members = mod.members;
@@ -96,11 +98,13 @@ void generateHtmlDocs(Path dst_path, Package root, GeneratorSettings settings =
        foreach (decl; members) {
            auto style = settings.fileNameStyle; // workaround for invalid value when directly used inside lamba
            auto name = decl.nestedName.splitter(".").map!(n => adjustMethodStyle(n, style)).join(".");
+           if (name != decl.nestedName)
+               rewriteMap.writeln("/", modpath ~ PathEntry(decl.nestedName~".html"), "\t", "/", modpath ~ PathEntry(name~".html"));
            auto pl = name in pages;
            if (pl && !canFind(*pl, decl.docGroup)) *pl ~= decl.docGroup;
            else if (!pl) pages[name] = [decl.docGroup];

-           collectChildren(decl, pages);
+           collectChildren(decl, pages, modpath);
        }
    }

@@ -114,7 +118,7 @@ void generateHtmlDocs(Path dst_path, Package root, GeneratorSettings settings =
        generateModulePage(file, root, mod, settings, ent => linkTo(ent, pack_path.length-dst_path.length));

        DocGroup[][string] pages;
-       collectChildren(mod, pages);
+       collectChildren(mod, pages, modpath.relativeTo(getWorkingDirectory() ~ "web"));
        foreach (name, decls; pages) {
            auto file = openFile(modpath ~ PathEntry(name~".html"), FileMode.createTrunc);
            scope(exit) file.close();

@MartinNowak
Copy link
Member Author

I checked that it works even though the RedirectMap isn't yet defined, this has to be done in the server config. Need to ask Jan for that.

@MartinNowak
Copy link
Member Author

Don't think it will have a bad impact on performance, it's just a hash table lookup (actually 2 because of the RewriteCond).

@CyberShadow
Copy link
Member

I checked that it works even though the RedirectMap isn't yet defined, this has to be done in the server config.

Hmm, that's not ideal... how about a PHP script as a 404 ErrorDocument for /library/?

@CyberShadow
Copy link
Member

Hmm, that's not ideal...

To clarify, as I understand that the patch as it is now will error out for all requests until the RedirectMap is added. That means that everyone who wants to host a mirror of the docs will need to modify their server configuration.

@MartinNowak
Copy link
Member Author

Not sure, it just won't perform the redirect, so it stays like it is right now.

@CyberShadow
Copy link
Member

Not sure, it just won't perform the redirect, so it stays like it is right now.

Sorry, what is that in reference to?

@MartinNowak
Copy link
Member Author

No, I just changed the .htaccess to test this, apache will ignore the non-existing map.
http://dlang.org/library/core/atomic/atomicFence.html

@MartinNowak
Copy link
Member Author

Sorry, what is that in reference to?

W0rp asked for those redirects, and I noticed myself, that you can easily land on dead pages from a google search.
http://forum.dlang.org/post/chpdvnhoyvghajmvengb@forum.dlang.org

@MartinNowak
Copy link
Member Author

Try outputrangeobject site:dlang.org for example.

@CyberShadow
Copy link
Member

Yes, I noticed that too, but Google will eventually de-index them.

@MartinNowak
Copy link
Member Author

Yes, I noticed that too, but Google will eventually de-index them.

But adding permanent redirects will help with that.

@CyberShadow
Copy link
Member

Not sure, it just won't perform the redirect, so it stays like it is right now.

I still don't understand what this means.

@MartinNowak
Copy link
Member Author

That means RewriteCond will be false and the rewrite won't happen until we add the map in the server config.

@CyberShadow
Copy link
Member

OK, let's carefully try this.

CyberShadow added a commit that referenced this pull request Apr 2, 2015
rewrites rules for dpl-docs file name style change
@CyberShadow CyberShadow merged commit eab3a6f into dlang:master Apr 2, 2015
@MartinNowak MartinNowak deleted the dpl_rewrites branch April 2, 2015 02:55
@MartinNowak
Copy link
Member Author

I'll update the website and mail Jan with detailed instructions.

@MartinNowak
Copy link
Member Author

Thanks

@CyberShadow
Copy link
Member

I'll update the website

Running an update now

@CyberShadow
Copy link
Member

(done)

@CyberShadow
Copy link
Member

@MartinNowak
Copy link
Member Author

I justed looked at the implementation of httpd (https://github.com/apache/httpd/blob/e7dc8e899a6a942e08784337e827c3772c6bb2b5/modules/mappers/mod_rewrite.c#L1272).
They say (https://httpd.apache.org/docs/current/rewrite/rewritemap.html#txt), that they do cache the lookup, but apparently do a per-line scan otherwise :(. This is still going to be fast (grep -qF text .dpl_rewrite_map.txt) takes less than 1ms, but we might consider to use an indexed file (https://httpd.apache.org/docs/current/rewrite/rewritemap.html#dbm) instead.

@CyberShadow
Copy link
Member

We should at least move this under /library/.htaccess so it doesn't impact the entire site.

@MartinNowak
Copy link
Member Author

Good point. Those dbm files are big, 6M, so we should create them on the server only.

@CyberShadow
Copy link
Member

Hmm...

dlang@k3:~$ du -hs d/dlang.org/web/
214M    d/dlang.org/web/

I don't think 6MB will make much of a difference.

@MartinNowak
Copy link
Member Author

I just don't want to check this into git.

du -h -d1 .git
56M .git

@CyberShadow
Copy link
Member

So don't, why not add a D tool that creates the .dbm file?

@CyberShadow
Copy link
Member

(disclaimer: no idea how difficult it actually is to do the above)

@CyberShadow
Copy link
Member

Let's just put that file directly on the server - it depends on a server-side configuration anyway. If possible, it should be outside /data/, so that any future rsync --delete doesn't delete it.

@MartinNowak
Copy link
Member Author

So don't, why not add a D tool that creates the .dbm file?

I'd rather sleep.

Let's just put that file directly on the server - it depends on a server-side configuration anyway. If possible, it should be outside /data/, so that any future rsync --delete doesn't delete it.

Right, let me figure out #952 first.

@MartinNowak
Copy link
Member Author

Rewrite rules are active.

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.

2 participants