Skip to content

Commit

Permalink
[gbc] Handle forward- and backslashes in paths
Browse files Browse the repository at this point in the history
Since .bond files can be authored on platforms with different directory
separator characters, the paths in build scripts and Bond `import`
statements may use a forwardslash when a backslash is expected or vice
versa.

In order to handle this, gbc now normalizes anything that is a
path (file paths, response file items, import directories, import
statement paths, output directories, &c.) to use the platform's
preferred directory separator character.

Tests for C++, C#, and Java have been updated to use some imports with
mixed slashes.

gbc doesn't use a dedicate type for file paths. `FilePath` is just a
type synonym for `String`, so some places may have been inadvertently
missed.

This does now mean that gbc cannot process files, on, say, Linux, with
backslashes in their names. This is expected to be rare, and is deemed
an acceptable compromise for a cross-platform tool.

Fixes #869
  • Loading branch information
chwarr committed May 5, 2018
1 parent 73f1861 commit 27e678f
Show file tree
Hide file tree
Showing 17 changed files with 70 additions and 21 deletions.
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ different versioning scheme, following the Haskell community's
* C++ codegen now applies the `--export-attribute` to the `ToString`,
`FromString`, `ToEnum` and `FromEnum` functions.
* `import` statements can now end with an optional semicolon.

* File and directory paths on the command line, in response files, or in
`import` statements can now use a mix of forward and backslashes. [Issue
#869](https://github.com/Microsoft/bond/issues/869)

### C++ ###

* **Breaking change** The deprecated Bond Comm functionality has been
Expand Down
18 changes: 16 additions & 2 deletions compiler/IO.hs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ module IO
, parseASTFile
, parseNamespaceMappings
, parseAliasMappings
, slashNormalize
)
where

Expand Down Expand Up @@ -54,9 +55,10 @@ parseBondFile importDirs file = do
Just path' -> do
content <- readFileUtf8 path'
return (path', content)
Nothing -> fail $ "Can't find import file " ++ importFile
Nothing -> fail $ "Can't find import file " ++ importFile'
where
findFilePath dirs = fmap (</> importFile) <$> firstM (doesFileExist . (</> importFile)) dirs
importFile' = slashNormalize importFile
findFilePath dirs = fmap (</> importFile') <$> firstM (doesFileExist . (</> importFile')) dirs

readFileUtf8 name = do
h <- openFile name ReadMode
Expand Down Expand Up @@ -102,3 +104,15 @@ combinedMessage err = id $ T.unpack $ T.intercalate (T.pack ", ") messages
-- parseErrorPretty returns a multi-line String.
-- We need to break it up to make a useful one-line message.
messages = T.splitOn (T.pack "\n") $ T.strip $ T.pack $ parseErrorTextPretty err

-- | Normalizes a file path to only use the current platform's preferred
-- directory separator.
--
-- Bond doesn't support files or directories with backslashes in their
-- names, so backslashes are always converted to the platform's preferred
-- separator.
slashNormalize :: FilePath -> FilePath
slashNormalize path = map replaceSlashes path
where replaceSlashes '/' = pathSeparator
replaceSlashes '\\' = pathSeparator
replaceSlashes c = c
21 changes: 19 additions & 2 deletions compiler/Options.hs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
-- Copyright (c) Microsoft. All rights reserved.
-- Licensed under the MIT license. See LICENSE file in the project root for full license information.

{-# LANGUAGE DeriveDataTypeable #-}
{-# LANGUAGE DeriveDataTypeable, RecordWildCards #-}
{-# OPTIONS_GHC -fno-warn-missing-fields #-}
{-# OPTIONS_GHC -fno-cse #-}

Expand All @@ -16,6 +16,7 @@ import Paths_bond (version)
import Data.Version (showVersion)
import System.Console.CmdArgs
import System.Console.CmdArgs.Explicit (processValue)
import IO (slashNormalize)

data ApplyOptions =
Compact |
Expand Down Expand Up @@ -128,14 +129,30 @@ schema = Schema
name "schema" &=
help "Output the JSON representation of the schema"

slashNormalizeOption :: Options -> Options
slashNormalizeOption Options = Options
slashNormalizeOption o@Cpp{..} = o { files = map slashNormalize files,
import_dir = map slashNormalize import_dir,
output_dir = slashNormalize output_dir }
slashNormalizeOption o@Cs{..} = o { files = map slashNormalize files,
import_dir = map slashNormalize import_dir,
output_dir = slashNormalize output_dir }
slashNormalizeOption o@Java{..} = o { files = map slashNormalize files,
import_dir = map slashNormalize import_dir,
output_dir = slashNormalize output_dir }
slashNormalizeOption o@Schema{..} = o { files = map slashNormalize files,
import_dir = map slashNormalize import_dir,
output_dir = slashNormalize output_dir }


mode :: Mode (CmdArgs Options)
mode = cmdArgsMode $ modes [cpp, cs, java, schema] &=
program "gbc" &=
help "Compile Bond schema file(s) and generate specified output. The schema file(s) can be in one of two formats: Bond IDL or JSON representation of the schema abstract syntax tree as produced by `gbc schema`. Multiple schema files can be specified either directly on the command line or by listing them in a text file passed to gbc via @listfile syntax." &=
summary ("Bond Compiler " ++ showVersion version ++ ", (C) Microsoft")

getOptions :: IO Options
getOptions = cmdArgsRun mode
getOptions = slashNormalizeOption <$> cmdArgsRun mode

processOptions :: [String] -> Options
processOptions = cmdArgsValue . processValue mode
18 changes: 13 additions & 5 deletions cpp/test/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ add_library (core_test_common
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/allocator_test_types.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/import_test1_types.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/import_test1_apply.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/import_test2_types.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/dir1/dir2/import_test2_types.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/apply_test_types.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/apply_test_apply.cpp"
"${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/validation_types.cpp"
Expand All @@ -50,7 +50,8 @@ add_dependencies(core_test_common
unit_test_codegen1
unit_test_codegen2
unit_test_codegen3
unit_test_codegen4)
unit_test_codegen4
unit_test_codegen_import2)
target_include_directories (core_test_common PUBLIC
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
Expand All @@ -64,10 +65,10 @@ target_link_libraries (core_test_common PUBLIC
bond
${Boost_UNIT_TEST_FRAMEWORK_LIBRARY})


add_bond_codegen (TARGET unit_test_codegen1
unit_test.bond
OPTIONS
--import-dir=imports
--using=\"static_string=std::array<char, {0}>\"
--using=\"static_wstring=std::array<wchar_t, {0}>\"
--using=\"simple_list=SimpleList<{0}>\"
Expand All @@ -78,13 +79,13 @@ add_bond_codegen (TARGET unit_test_codegen2
unit_test_core.bond
apply_test.bond
import_test1.bond
import_test2.bond
scope_test1.bond
scope_test2.bond
validation.bond
cmdargs.bond
OPTIONS
--header=\\\"custom_protocols.h\\\")
--import-dir=imports
--header=\\\"custom_protocols.h\\\")

add_bond_codegen (TARGET unit_test_codegen3
allocator_test.bond
Expand All @@ -103,6 +104,13 @@ add_bond_codegen (TARGET unit_test_codegen4
--allocator=\"bond::capped_allocator<>\"
--namespace=\"allocator_test=capped_allocator_tests\")

add_bond_codegen (TARGET unit_test_codegen_import2
imports/dir1/dir2/import_test2.bond
# Need a custom output path so the generated #include paths line up
OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/dir1/dir2
OPTIONS
--header=\\\"custom_protocols.h\\\")

# Full unit test is too big, .pdb files exceed compiler limit;
# Since debugging such large executable is not practical anyways,
# we disable .pdb generation.
Expand Down
2 changes: 1 addition & 1 deletion cpp/test/core/apply_test.bond
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
///

import "import_test1.bond"
import "import_test2.bond"
import "dir1/dir2/import_test2.bond"

namespace unittest.apply;

Expand Down
4 changes: 2 additions & 2 deletions cpp/test/core/import_test1.bond
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import "import_test2.bond"
// Uses mixed slashes to test gbc can deal with that
import "dir1/dir2\import_test2.bond"

namespace import1
namespace csharp Microsoft.Import1
Expand All @@ -8,4 +9,3 @@ struct ImportedStruct
10: required_optional int32 x;
20: required_optional import2.ImportedEnum e = ImportedEnum2;
};

File renamed without changes.
2 changes: 1 addition & 1 deletion cpp/test/core/unit_test.bond
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
///

import "import_test1.bond"
import "import_test2.bond"
import "dir1/dir2/import_test2.bond"
import "unit_test_core.bond"

namespace unittest;
Expand Down
2 changes: 1 addition & 1 deletion cs/dnc/test/core.tests/core.tests.codegen.proj
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
<BondCodegen Include="..\..\..\test\core\UnitTest.bond">
<Options>$(BondOptions) --using="DateTime=System.DateTime"</Options>
</BondCodegen>
<BondCodegen Include="..\..\..\test\core\import dir with spaces\Bond File With Spaces.bond" />
<BondCodegen Include="..\..\..\test\core\import dir with spaces\dir1\dir2\Bond File With Spaces.bond" />
</ItemGroup>
<Import Project="$(MSBuildThisFileDirectory)\..\..\..\build\nuget\Common.targets" />
</Project>
5 changes: 3 additions & 2 deletions cs/test/core/Core.csproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath32)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath32)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<PropertyGroup Condition="'$(BuildNonportable)' == 'true'">
Expand Down Expand Up @@ -75,7 +75,8 @@
<BondCodegen Include="UnitTest.bond">
<Options>$(BondOptions) --using="DateTime=System.DateTime"</Options>
</BondCodegen>
<BondCodegen Include="import dir with spaces\Bond File With Spaces.bond" />
<!-- uses mixed slashes to test the targets can deal with that -->
<BondCodegen Include="import dir with spaces\dir1/dir2/Bond File With Spaces.bond" />
<!-- Resharper Workaround -->
<Compile Include="$(IntermediateOutputPath)\Aliases_types.cs" Condition="False" />
<Compile Include="$(IntermediateOutputPath)\Bond File With Spaces_types.cs" Condition="False" />
Expand Down
3 changes: 2 additions & 1 deletion cs/test/core/UnitTest.bond
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import "bond/core/bond.bond"
import "Aliases.bond"
import "Bond File With Spaces.bond"
// Uses mixed slashes to test gbc can deal with that
import "dir1\dir2/Bond File With Spaces.bond"

namespace UnitTest

Expand Down
4 changes: 4 additions & 0 deletions java/core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,7 @@ compileBond {
'../../idl/bond/core/bond_const.bond'
options '-n', 'bond=org.bondlib'
}

compileTestBond {
options '--import-dir', 'src/test/bond/imports/'
}
3 changes: 2 additions & 1 deletion java/core/src/test/bond/bonded.bond
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import "common.bond"
// Uses mixed slashes to test gbc can deal with that
import "dir1\dir2/common.bond"

namespace org.bondlib.test

Expand Down
2 changes: 1 addition & 1 deletion java/core/src/test/bond/equality.bond
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import "common.bond"
import "dir1/dir2/common.bond"

namespace org.bondlib.test

Expand Down
2 changes: 1 addition & 1 deletion java/core/src/test/bond/generic.bond
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import "common.bond"
import "dir1/dir2/common.bond"

namespace org.bondlib.test

Expand Down
File renamed without changes.

0 comments on commit 27e678f

Please sign in to comment.