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

Resolving files relative to sourceDir (Issue in 1.5.1-SNAPSHOT) #122

Closed
ysb33r opened this issue Sep 9, 2014 · 7 comments
Closed

Resolving files relative to sourceDir (Issue in 1.5.1-SNAPSHOT) #122

ysb33r opened this issue Sep 9, 2014 · 7 comments
Milestone

Comments

@ysb33r
Copy link
Member

ysb33r commented Sep 9, 2014

This is more of a question and it is about structuring files and it has been confusing me the last couple of days. We'll need to update the README with the correct details so here goes

Given a file structure

projectDir / src / elsewhere / file1.adoc
projectDir / src / asciidoc / file2.adoc

and

  sourceDir = file('src/asciidoc')

Which of the following 5 are valid?

  // [1] File on it's own should be be interpreted as having sourceDir as parent?
  sourceDocumentNames 'file2.adoc' 
  // [2] Absolute file is fine as long as it's root is still sourceDir?
  sourceDocumentNames new File(projectDir,'src/asciidoc/file2.adoc').absoluteFile
 // [3] Relative file reachable from sourceDir, but not having sourceDir as parent?
 sourceDocumentNames '../elsewhere/file1.adoc'
 // [4] Absolute file, not having sourceDIr as parent but still reachable from sourceDir?
 sourceDocumentNames new File(projectDir,'src/elsewhere/file1.adoc').absoluteFile
// [5] Assuming a multi-root system such as Windows with projectDir located on D:
  sourceDocumentNames 'E:/another.adoc'

From my understanding

  • No valid: [5]
  • Valid: [1], [2]
  • Unsure: [3], [4]

Would like to hear what @aalmiray has to say on this one.

@ysb33r
Copy link
Member Author

ysb33r commented Sep 9, 2014

I think this section of code in Asciidoctortask effectively states that [3], [4] above are not valid. It also effectively states that [2] is acceptable, but frowned upon.

   private void validateSourceDocuments(FileCollection srcDocs) {
        srcDocs.files.findAll {
            it.isAbsolute() && !it.canonicalPath.startsWith(sourceDir.canonicalPath)
        }.each {
            logger.warn("Entry '$it' of `sourceDocumentNames` should be specified relative to `sourceDir` ($sourceDir)")
        }
        Collection<File> allReachableFiles = []
        eachFileRecurse(sourceDir, EXCLUDE_DOCINFO_AND_FILES_STARTING_WITH_UNDERSCORE) { File file ->
            allReachableFiles.add(file)
        }
        srcDocs.files.each { File file ->
            if (! allReachableFiles.find {it.canonicalPath == file.canonicalPath}) {
                throw new GradleException("'$file' is not reachable from sourceDir ($sourceDir). " +
                        'All files given in `sourceDocumentNames` must be located in `sourceDir` ' +
                        'or a subfolder of `sourceDir`.')
            }
        }
    }

Based upon this, I think there is an issue in 1.5.1-SNAPSHOT that was introduced by #118. By example this following snippet will not work

asciidoctor {
  sourceDir 'src/asciidoc'
  sourceDocumentNames 'slides.adoc'
}

It resolves to ${projectDir}/slides.adoc instead of ${projectDir}/src/asciidoc/slides.adoc.

@ysb33r ysb33r changed the title Resolving files relative to sourceDir Resolving files relative to sourceDir (Issue in 1.5.1-SNAPSHOT) Sep 9, 2014
@mojavelinux
Copy link
Member

The slides.adoc example you gave should definitely resolve to ${projectDir}/src/asciidoc/slides.adoc. The sourceDir is the search root for files.

I do think that cases 1-4 are all reasonable to support. 5 should be frowned upon because it makes for a non-portable build (referencing an external file). However, we don't have to support all the cases 1 - 4. We should support the cases that are necessary based on user requirements / feedback.

@mojavelinux
Copy link
Member

I want to address the question about why the sourceDocumentNames are relative to the sourceDir, and why we have sourceDir.

The reason we have sourceDir is to put AsciidoctorJ (and thus Asciidoctor) into the context of the documentation folder, similar to if we were using the asciidoctor command. It's the relative documentation root. If we assumed the root to be the project directory, then it would force authors to use paths that bind the documentation to the environment of the Gradle build. That would end up breaking preview when using external tools like the Chrome extension.

The challenge we currently face with regard to this relative root is that Asciidoctor is currently trying to represent two concepts with only a single option. I'll explain.

Currently, the only way to define the "root" directory in Asciidoctor is by setting the base_dir option. However, Asciidoctor is also using the base_dir as the chroot environment (i.e., the jail). This means that if you set the base_dir and you are using a safe mode > unsafe, then you can't reference any files outside of this root. It's perfectly reasonable, however, to reference files anywhere in the project.

In Asciidoctor core, we need to split base_dir into two options: base_dir and jail_dir (or chroot). That allows us to define the confined space in which Asciidoctor should work, but also to set a root directory for resolving things like includes. Then, this becomes a much simpler problem in build environments like Gradle. See asciidoctor/asciidoctor#682

As a general word of advice, you want to avoid setting a root (i.e., base_dir) as the project dir because then it breaks the AsciiDoc file preview when viewed in any other context (such as in the Chrome extension). We don't want to couple the AsciiDoc files to the Gradle build in that way...so it's important to use the most universal idea of a base directory and then work from there.

This is also one of the main reasons we advocate for always prefixing the include target with a variable, such as {projectdir}, etc. Then, you can control the resolution of the targets in a much more fine-grained way.

@aalmiray
Copy link
Member

I'm with @mojavelinux here, the slides.adoc should be resolved relative to sourceDir. If it does not then that's a bug we must fix.

@ysb33r
Copy link
Member Author

ysb33r commented Sep 10, 2014

I'm with @mojavelinux here, the slides.adoc should be resolved relative to sourceDir. If it does not then that's a bug we must fix

I have a fix for that; will create a PR.

@ysb33r
Copy link
Member Author

ysb33r commented Sep 10, 2014

As a general word of advice, you want to avoid setting a root (i.e., base_dir) as the project dir because then it breaks the AsciiDoc file preview when viewed in any other context (such as in the Chrome extension). We don't want to couple the AsciiDoc files to the Gradle build in that way...so it's important to use the most universal idea of a base directory and then work from there.

If I understand this correctly, the true aim is not to restrict people in the way they build docuemtns with Asciidoctor. Someone who also uses the chrome extension would want to have everything below sourceDir including images etc. Therefore trying to force a style, for example, as putting docs under src/asciidoc and images under src/main/resources in gradle is not helpful.

Consider an alternative use-case using my gist on including deck.js without git submodules as example. In this case deck.js is unpacked in a subfolder of buildDir and then copied to AsciidoctorTask.outputDir at some stage. Does this matter to Asciidoctor in safe mode? Probably not. Does it matter to someone using the Chrome extension? I would think not, as the extension would not know about the alternative template_dir in use.

So I would guess that what needs to be descendents of sourceDir are all of the Asciidoc files required by Asciidoctor i order to generate the required output. So in my example above the deck.js files are not required - they are only used at display time. This is similar to the way MathJax works at the moment anyway. A stub is generated into the document that knows how to access MathJax ar display time.

This really brings me to another thing - images (videos etc.). I suspect convention is just to have that as a subfolder of sourceDir and then copy it across to the outputDir as part of the build. The lines

File target = new File(destinationParentDir, file.name)
target.withOutputStream { it << file.newInputStream() }

in Asciidoctor.processSourceDir alludes to this anyway. Does Asciidoctor require images to be descendents of sourceDir?

@mojavelinux
Copy link
Member

Does Asciidoctor require images to be descendents of sourceDir?

Not necessarily. Image paths are determines by the imagesdir attribute, which is blank by default. But images are complex because it really depends on whether you are linked to or embedding the image. If you are embedding, it tries to find the image at the time of conversion at a path relative to the source directory (depending on the value of the imagesdir attribute). If you are linking to the image, then it just puts the path to the image in the HTML and only the browser attempts to locate it. That's why images can be rather confusing.

aalmiray added a commit that referenced this issue Sep 25, 2014
@aalmiray aalmiray added this to the 1.5.2 milestone Dec 8, 2014
@aalmiray aalmiray closed this as completed Dec 8, 2014
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

No branches or pull requests

3 participants