[PATCH] RFC: New "apitrace diff" command.
José Fonseca
jose.r.fonseca at gmail.com
Thu Nov 3 15:51:43 PDT 2011
Carl,
The comments I have are pretty much the same I did jjust before:
wrapper DLLs must be on a directory of its own. Everything else
(scripts progs) can be lumped together. That is, please keep
APITRACE_WRAPPER_INSTALL_DIR and LIB_INSTALL_DIR separate.
I've already moved file_exists to os_path.hpp as a os::Path method.
The rest looks good.
BTW, tracediff.sh is one of the scripts that I need to rewrite. It's
too slow, and non portable.
Jose
On Thu, Nov 3, 2011 at 12:31 AM, Carl Worth <cworth at cworth.org> wrote:
> This is a requset-for-comments commit.
>
> The stuff changing the name of the define for the install directory should
> obviously be a separate commit, for example.
>
> And the stubbed out support for the Windows platform should be fixed as well.
>
> This is one example of a command that merely executes an existing,
> external script.
>
> I would be happy for any comments.
> ---
>
> So, obviously not ready to commit in its current form, but I thought I
> would at least let you see what I'm working on.
>
> This is on top of the previously-mentioned apitrace-trace branch, and
> is also available in the rfc-apitrace-diff branch of my repository.
>
> -Carl
>
> CMakeLists.txt | 25 +++++++-----
> cli/CMakeLists.txt | 2 +
> cli/cli.hpp | 5 ++
> cli/cli_common.cpp | 59 ++++++++++++++++++++++++++++
> cli/cli_diff.cpp | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++
> cli/cli_main.cpp | 1 +
> cli/cli_trace.cpp | 47 ----------------------
> common/os.hpp | 3 +
> common/os_posix.cpp | 16 ++++++++
> common/os_win32.cpp | 8 ++++
> gui/traceprocess.cpp | 4 +-
> scripts/tracediff.sh | 2 +-
> 12 files changed, 218 insertions(+), 60 deletions(-)
> create mode 100644 cli/cli_common.cpp
> create mode 100644 cli/cli_diff.cpp
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 6caf62e..d659efa 100755
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -194,9 +194,9 @@ endif ()
> if (APPLE)
> # MacOSX uses fat binaries, so no need to have per-architecture wrapper
> # directories
> - set (WRAPPER_INSTALL_DIR lib/apitrace)
> + set (LIB_INSTALL_DIR lib/apitrace)
> else ()
> - set (WRAPPER_INSTALL_DIR lib/apitrace/${CMAKE_SYSTEM_PROCESSOR})
> + set (LIB_INSTALL_DIR lib/apitrace/${CMAKE_SYSTEM_PROCESSOR})
> endif ()
>
> # Expose the binary/install directories to source
> @@ -207,7 +207,7 @@ endif ()
> add_definitions(
> -DAPITRACE_BINARY_DIR="${CMAKE_BINARY_DIR}"
> -DAPITRACE_INSTALL_PREFIX="${CMAKE_INSTALL_PREFIX}"
> - -DAPITRACE_WRAPPER_INSTALL_DIR="${CMAKE_INSTALL_PREFIX}/${WRAPPER_INSTALL_DIR}"
> + -DAPITRACE_LIB_INSTALL_DIR="${CMAKE_INSTALL_PREFIX}/${LIB_INSTALL_DIR}"
> )
>
>
> @@ -285,7 +285,7 @@ if (WIN32)
> RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> )
> - install (TARGETS ddraw LIBRARY DESTINATION ${WRAPPER_INSTALL_DIR})
> + install (TARGETS ddraw LIBRARY DESTINATION ${LIB_INSTALL_DIR})
> endif (DirectX_D3D_INCLUDE_DIR)
>
> # d3d8.dll
> @@ -302,7 +302,7 @@ if (WIN32)
> RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> )
> - install (TARGETS d3d8 LIBRARY DESTINATION ${WRAPPER_INSTALL_DIR})
> + install (TARGETS d3d8 LIBRARY DESTINATION ${LIB_INSTALL_DIR})
> endif (DirectX_D3D8_INCLUDE_DIR AND DirectX_D3DX9_INCLUDE_DIR)
>
> # d3d9.dll
> @@ -319,7 +319,7 @@ if (WIN32)
> RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> )
> - install (TARGETS d3d9 LIBRARY DESTINATION ${WRAPPER_INSTALL_DIR})
> + install (TARGETS d3d9 LIBRARY DESTINATION ${LIB_INSTALL_DIR})
> endif (DirectX_D3DX9_INCLUDE_DIR)
>
> # d3d10.dll
> @@ -336,7 +336,7 @@ if (WIN32)
> RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> )
> - install (TARGETS d3d10 LIBRARY DESTINATION ${WRAPPER_INSTALL_DIR})
> + install (TARGETS d3d10 LIBRARY DESTINATION ${LIB_INSTALL_DIR})
> endif (DirectX_D3D10_INCLUDE_DIR)
>
> # opengl32.dll
> @@ -356,7 +356,7 @@ if (WIN32)
> RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> LIBRARY_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/wrappers
> )
> - install (TARGETS wgltrace LIBRARY DESTINATION ${WRAPPER_INSTALL_DIR})
> + install (TARGETS wgltrace LIBRARY DESTINATION ${LIB_INSTALL_DIR})
>
> elseif (APPLE)
> # OpenGL framework
> @@ -383,7 +383,7 @@ elseif (APPLE)
>
> target_link_libraries (cgltrace dl)
>
> - install (TARGETS cgltrace LIBRARY DESTINATION ${WRAPPER_INSTALL_DIR})
> + install (TARGETS cgltrace LIBRARY DESTINATION ${LIB_INSTALL_DIR})
> else ()
> # libGL.so
> add_custom_command (
> @@ -412,7 +412,7 @@ else ()
>
> target_link_libraries (glxtrace dl ${X11_X11_LIB})
>
> - install (TARGETS glxtrace LIBRARY DESTINATION ${WRAPPER_INSTALL_DIR})
> + install (TARGETS glxtrace LIBRARY DESTINATION ${LIB_INSTALL_DIR})
> endif ()
>
>
> @@ -476,6 +476,11 @@ endif ()
> install (TARGETS glretrace RUNTIME DESTINATION bin)
>
> ##############################################################################
> +# Scripts
> +
> +install (PROGRAMS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/tracediff.sh DESTINATION ${LIB_INSTALL_DIR}/scripts)
> +
> +##############################################################################
> # CLI
>
> add_subdirectory(cli)
> diff --git a/cli/CMakeLists.txt b/cli/CMakeLists.txt
> index fc52527..b9f2c50 100644
> --- a/cli/CMakeLists.txt
> +++ b/cli/CMakeLists.txt
> @@ -1,5 +1,7 @@
> add_executable (apitrace
> cli_main.cpp
> + cli_common.cpp
> + cli_diff.cpp
> cli_dump.cpp
> cli_trace.cpp
> )
> diff --git a/cli/cli.hpp b/cli/cli.hpp
> index 9ab1600..9439216 100644
> --- a/cli/cli.hpp
> +++ b/cli/cli.hpp
> @@ -28,6 +28,7 @@
> #ifndef _APITRACE_CLI_HPP_
> #define _APITRACE_CLI_HPP_
>
> +#include "os_path.hpp"
>
> struct Command {
> const char *name;
> @@ -40,7 +41,11 @@ struct Command {
> Function function;
> };
>
> +extern const Command diff_command;
> extern const Command dump_command;
> extern const Command trace_command;
>
> +os::Path
> +find_private_binary(const char *filename);
> +
> #endif /* _APITRACE_CLI_HPP_ */
> diff --git a/cli/cli_common.cpp b/cli/cli_common.cpp
> new file mode 100644
> index 0000000..a2a63af
> --- /dev/null
> +++ b/cli/cli_common.cpp
> @@ -0,0 +1,59 @@
> +/*********************************************************************
> + *
> + * Copyright 2011 Intel Corporation
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use, copy,
> + * modify, merge, publish, distribute, sublicense, and/or sell copies
> + * of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + *********************************************************************/
> +
> +#include <iostream>
> +
> +#include "cli.hpp"
> +
> +os::Path
> +find_private_binary(const char *filename)
> +{
> + /* First look in the same directory from which this process is
> + * running, (to support developers running a compiled program that
> + * has not been installed. */
> + os::Path process_dir = os::getProcessName();
> +
> + process_dir.trimFilename();
> +
> + os::Path complete = process_dir;
> + complete.join(filename);
> +
> + if (os::fileExists(complete))
> + return complete;
> +
> + /* Second, look in the directory for installed wrappers. */
> + complete = APITRACE_LIB_INSTALL_DIR;
> + complete.join(filename);
> +
> + if (os::fileExists(complete))
> + return complete;
> +
> + std::cerr << "Error: Cannot find " << filename << " (looked in " <<
> + APITRACE_LIB_INSTALL_DIR << ").\n";
> +
> + return "";
> +}
> diff --git a/cli/cli_diff.cpp b/cli/cli_diff.cpp
> new file mode 100644
> index 0000000..550e813
> --- /dev/null
> +++ b/cli/cli_diff.cpp
> @@ -0,0 +1,106 @@
> +/*********************************************************************
> + *
> + * Copyright 2011 Intel Corporation
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use, copy,
> + * modify, merge, publish, distribute, sublicense, and/or sell copies
> + * of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + *********************************************************************/
> +
> +#include <string.h>
> +#include <iostream>
> +
> +#include "cli.hpp"
> +
> +static const char *synopsis = "Identify differences between two traces.";
> +
> +static void
> +usage(void)
> +{
> + std::cout
> + << "usage: apitrace diff <trace-1> <trace-2>\n"
> + << synopsis << "\n"
> + "\n"
> + " Both input files should be the result of running 'apitrace trace'.\n";
> +}
> +
> +static int
> +command(int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < argc; ++i) {
> + const char *arg = argv[i];
> +
> + if (arg[0] != '-') {
> + break;
> + }
> +
> + if (!strcmp(arg, "--")) {
> + i++;
> + break;
> + } else if (!strcmp(arg, "--help")) {
> + usage();
> + return 0;
> + } else {
> + std::cerr << "error: unknown option " << arg << "\n";
> + usage();
> + return 1;
> + }
> + }
> +
> + if (argc - i != 2) {
> + std::cerr << "Error: diff requires exactly two trace files as arguments.\n";
> + usage();
> + return 1;
> + }
> +
> + char *file1, *file2;
> +
> + file1 = argv[i];
> + file2 = argv[i+1];
> +
> + os::Path command = find_private_binary("scripts/tracediff.sh");
> +
> + char* args[4];
> +
> + args[0] = (char *) command.str();
> + args[1] = file1;
> + args[2] = file2;
> + args[3] = NULL;
> +
> +#ifdef _WIN32
> + std::cerr << "The 'apitrace diff' command is not yet supported on this O/S.\n";
> +#else
> + execv(command.str(), args);
> +#endif
> +
> + std::cerr << "Error: Failed to execute " << argv[0] << "\n";
> +
> + return 1;
> +}
> +
> +const Command diff_command = {
> + "diff",
> + synopsis,
> + usage,
> + command
> +};
> diff --git a/cli/cli_main.cpp b/cli/cli_main.cpp
> index a3abf54..421748c 100644
> --- a/cli/cli_main.cpp
> +++ b/cli/cli_main.cpp
> @@ -66,6 +66,7 @@ const Command help_command = {
> };
>
> static const Command * commands[] = {
> + &diff_command,
> &dump_command,
> &trace_command,
> &help_command
> diff --git a/cli/cli_trace.cpp b/cli/cli_trace.cpp
> index 4eb9a15..0da08ed 100644
> --- a/cli/cli_trace.cpp
> +++ b/cli/cli_trace.cpp
> @@ -29,8 +29,6 @@
>
> #include "cli.hpp"
>
> -#include "os_path.hpp"
> -
> static const char *synopsis = "Generate a new trace by executing the given program.";
>
> static void
> @@ -59,51 +57,6 @@ usage(void)
> #endif
>
> static int
> -file_exists(const char *path)
> -{
> - struct stat st;
> - int err;
> -
> - err = stat(path, &st);
> - if (err)
> - return 0;
> -
> - if (! S_ISREG(st.st_mode))
> - return 0;
> -
> - return 1;
> -}
> -
> -static os::Path
> -find_private_binary(const char *filename)
> -{
> - /* First look in the same directory from which this process is
> - * running, (to support developers running a compiled program that
> - * has not been installed. */
> - os::Path process_dir = os::getProcessName();
> -
> - process_dir.trimFilename();
> -
> - os::Path complete = process_dir;
> - complete.join(filename);
> -
> - if (file_exists(complete))
> - return complete;
> -
> - /* Second, look in the directory for installed wrappers. */
> - complete = APITRACE_WRAPPER_INSTALL_DIR;
> - complete.join(filename);
> -
> - if (file_exists(complete))
> - return complete;
> -
> - std::cerr << "Error: Cannot find " << filename << " (looked in " <<
> - APITRACE_WRAPPER_INSTALL_DIR << ").\n";
> -
> - return "";
> -}
> -
> -static int
> do_trace_posix(int argc, char *argv[])
> {
> os::Path binary = find_private_binary(CLI_TRACE_BINARY);
> diff --git a/common/os.hpp b/common/os.hpp
> index 6a3b8c8..24e3b1f 100644
> --- a/common/os.hpp
> +++ b/common/os.hpp
> @@ -84,6 +84,9 @@ void abort(void);
> void setExceptionCallback(void (*callback)(void));
> void resetExceptionCallback(void);
>
> +
> +int fileExists(const char *path);
> +
> } /* namespace os */
>
> #endif /* _OS_HPP_ */
> diff --git a/common/os_posix.cpp b/common/os_posix.cpp
> index 21a7e36..9ea531a 100644
> --- a/common/os_posix.cpp
> +++ b/common/os_posix.cpp
> @@ -245,5 +245,21 @@ resetExceptionCallback(void)
> gCallback = NULL;
> }
>
> +int
> +fileExists(const char *path)
> +{
> + struct stat st;
> + int err;
> +
> + err = stat(path, &st);
> + if (err)
> + return 0;
> +
> + if (! S_ISREG(st.st_mode))
> + return 0;
> +
> + return 1;
> +}
> +
> } /* namespace os */
>
> diff --git a/common/os_win32.cpp b/common/os_win32.cpp
> index f33e175..f2c07b9 100644
> --- a/common/os_win32.cpp
> +++ b/common/os_win32.cpp
> @@ -25,6 +25,7 @@
>
> #include <windows.h>
>
> +#include <iostream>
> #include <assert.h>
> #include <string.h>
> #include <stdio.h>
> @@ -180,5 +181,12 @@ resetExceptionCallback(void)
> gCallback = NULL;
> }
>
> +int
> +fileExists(const char *path)
> +{
> + std::cerr << "Internal error: No support yet for fileExists() on this O/S.\n";
> +
> + return 0;
> +}
>
> } /* namespace os */
> diff --git a/gui/traceprocess.cpp b/gui/traceprocess.cpp
> index 6bdd1c4..a648441 100644
> --- a/gui/traceprocess.cpp
> +++ b/gui/traceprocess.cpp
> @@ -18,8 +18,8 @@ findPreloader()
> return libPath;
> #endif
>
> -#ifdef APITRACE_WRAPPER_INSTALL_DIR
> - libPath = QString::fromLatin1(APITRACE_WRAPPER_INSTALL_DIR "/glxtrace.so");
> +#ifdef APITRACE_LIB_INSTALL_DIR
> + libPath = QString::fromLatin1(APITRACE_LIB_INSTALL_DIR "/glxtrace.so");
> fi = QFileInfo(libPath);
> if (fi.exists())
> return libPath;
> diff --git a/scripts/tracediff.sh b/scripts/tracediff.sh
> index b3b9220..57d59f8 100755
> --- a/scripts/tracediff.sh
> +++ b/scripts/tracediff.sh
> @@ -26,7 +26,7 @@
>
> set -e
>
> -APITRACE=${APITRACE:-`dirname "$0"`/../apitrace}
> +APITRACE=${APITRACE:-apitrace}
>
> $APITRACE dump
>
> --
> 1.7.7
>
> _______________________________________________
> apitrace mailing list
> apitrace at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/apitrace
>
More information about the apitrace
mailing list