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

Conflict between implementation_deps and deps when using target mapping #253

Closed
TendTo opened this issue May 20, 2024 · 2 comments · Fixed by #254
Closed

Conflict between implementation_deps and deps when using target mapping #253

TendTo opened this issue May 20, 2024 · 2 comments · Fixed by #254

Comments

@TendTo
Copy link

TendTo commented May 20, 2024

I'm not sure if the issue I'm encountering depends on some miscofiguration on my part or it is a bug in the tool, but I'll report it here.
There are some configurations, even very simple ones, where using the use_implementation_deps=True flag creates unsatisfaiable requirements for dwyu if a target map is specified, even when the map itself is completely empty.

What follows is a simple example i was able to prepare

# MODULE.bazel
module(name = "example")

bazel_dep(name = "rules_cc", version = "0.0.9")
bazel_dep(name = "fmt", version = "10.2.1")
bazel_dep(name = "spdlog", version = "1.12.0")
bazel_dep(name = "depend_on_what_you_use", version = "0.0.0")
git_override(
    module_name = "depend_on_what_you_use",
    commit = "b817c225d79c492c25b28872445e34be4a36d5aa",
    remote = "https://github.com/martis42/depend_on_what_you_use",
)
# BUILD.bazel
load("@depend_on_what_you_use//:defs.bzl", "MAP_TRANSITIVE_DEPS", "dwyu_make_cc_info_mapping")

package(default_visibility = ["//visibility:public"])

exports_files([
    "ignore_includes.json",
])

# DWYU mappings
dwyu_make_cc_info_mapping(
    name = "dwyu_mapping",
    mapping = {
# Empty mapping
    },
)
# dwyu.bzl
load("@depend_on_what_you_use//:defs.bzl", "dwyu_aspect_factory")

dwyu = dwyu_aspect_factory(use_implementation_deps = True, recursive = True, skip_external_targets = True, target_mapping = Label("//:dwyu_mapping"))
# main/BUILD.bazel
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")

cc_library(
    name = "logging",
    hdrs = ["logging.h"],
    deps = ["@fmt", "@spdlog"],
)

cc_library(
    name = "lib",
    srcs = ["lib.cpp"],
    hdrs = ["lib.h"],
    # Commenting one and uncommenting the other does not satisfy dwyu
    # implementation_deps = [":logging"], 
    deps = [":logging"],
)

cc_binary(
    name = "main",
    srcs = ["main.cpp"],
    deps = [":lib"],
)
//----------------------------
// In main/lib.h

// Empty

//----------------------------
// In main/lib.cpp

#include "lib.h"
#include "logging.h"

//----------------------------
// In main/logging.h

#ifndef NLOG
#include <fmt/core.h>
#include <spdlog/spdlog.h>
#else
#endif

//----------------------------
// In main/main.cpp

#include "lib.h"

Running

bazel build --aspects=//:dwyu.bzl%dwyu --output_groups=dwyu //main

Output if using deps

================================================================================
DWYU analyzing: '@@//main:lib'

Result: FAILURE

Public dependencies which are used only in private code:
  Dependency='@@//main:logging'
================================================================================

Output if using implementation_deps

================================================================================
DWYU analyzing: '@@//main:lib'

Result: FAILURE

Includes which are not available from the direct dependencies:
  File='main/lib.cpp', include='logging.h'
================================================================================
Aspect //:dwyu.bzl%dwyu of //main:main failed to build

EDIT:

bazel version 7.1.1
@martis42
Copy link
Owner

Thanks for this detailed report.
Unfortunately I am currently busy with other things and will not be able to look into this immediately. But will manage to do so eventually and then come back to you.

@martis42
Copy link
Owner

@TendTo again thanks for providing such a good case for reproducing 🤩
Like this it was a piece of cake to find the bug (which actually is kinda embarrassing as I simply forgot something 😅)

The fix should merge soonish, see #254

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 a pull request may close this issue.

2 participants