- 
                Notifications
    You must be signed in to change notification settings 
- Fork 125
Assert that only declared dependencies are used when compiling/linking #466
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As it stands, I need to use this from the introduced TaskDependencyVerification.Adapter.exec() (volatile link), so made it static. This doesn't feel right.
A better home might be as an extension of the newly introduced TaskExecutionContext (volatile link). That's an even more invasive change though.
Any suggestions on how to clean this up welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving to TaskExecutionContext seems fine; there's no particularly strong reason I put this here.
        
          
                Sources/SWBTaskExecution/TaskActions/ClangCompileTaskAction.swift
              
                Outdated
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Libraries on Windows don't start with lib. This is also not technically required on Unix either, so we should think how to handle mapping of libraries that aren't necessarily using the lib prefix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've more deeply encapsulated this and added clarifying documentation that it does not attempt to be a viable implementation for general use.
The intention is not to do more with this at this time in this change set, but evolve later.
How do you suggest we resolve the partial status of this for this PR?
570e1c2    to
    7699bd5      
    Compare
  
    | @swift-ci test | 
| @mirza-garibovic @jakepetroules this is ready for another review pass. I don't expect that it is mergeable, but there is nothing that I intend to add change other than addressing feedback-to-come. | 
0cd961d    to
    73e0bd7      
    Compare
  
    | @swift-ci test | 
dea823c    to
    20f376b      
    Compare
  
    | @swift-ci test | 
    
      
        1 similar comment
      
    
  
    | @swift-ci test | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
d7e3460    to
    36b24d6      
    Compare
  
    | @swift-ci test macos | 
| @swift-ci test | 
11610fc    to
    2209f09      
    Compare
  
    …-time This is a skeletal implementation scoped to support a specific functional milestone.
Also adds disclaimer that implementation is provisional.
2209f09    to
    2338967      
    Compare
  
    | @swift-ci test | 
| @swift-ci test | 
This is the first increment towards supporting centralized declared dependencies.
This introduces a
DEPENDENCIESbuild setting, that is a string list of “logical” dependency names (e.g.FooforFoo.framework). When invoking a tool that supports trace information, the trace will be cross referenced with the declared dependencies and the action will fail if a file is used that does not “resolve” to a logical dependency declared inDEPENDENCIES. This process of analyzing trace data and asserting that only declared dependencies are used is referred to as “verification”.This PR adds verification for linking, and non-modular clang compilation. Later changes will add support for other trace-producing tools.
The error reporting and diagnostic experience in this change is considered rudimentary/provisional, and will be improved in subsequent changes.