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.

Sign up for GitHub

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

Jump to bottom

Move the remaining crates to separate workspaces #1122

Merged
merged 1 commit into from
Jun 11, 2020
Merged

Move the remaining crates to separate workspaces #1122

merged 1 commit into from
Jun 11, 2020

Conversation

rbehjati
Copy link
Contributor

@rbehjati rbehjati commented Jun 9, 2020 •
edited
Loading

Ref #971

Checklist

  • Pull request affects core Oak functionality (e.g. runtime, SDK, ABI)
    • I have written tests that cover the code changes.
    • I have checked that these tests are run by
      Cloudbuild
    • I have updated documentation accordingly.
    • I have raised an issue to
      cover any TODOs and/or unfinished work.
  • Pull request includes prototype/experimental work that is under
    construction.

@rbehjati rbehjati added the WIP Work in progress label Jun 9, 2020
@rbehjati rbehjati force-pushed the oak-971-rest branch 2 times, most recently from 14b65bd to b6e2075 Compare June 9, 2020 18:11
@rbehjati rbehjati removed the WIP Work in progress label Jun 9, 2020
tiziano88
tiziano88 reviewed Jun 9, 2020
View reviewed changes
Cargo.toml
"third_party/roughenough",
]
exclude = ["oak_abi", "oak_utils"]
members = ["oak/server/rust/oak_loader", "oak/server/rust/oak_runtime"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels we should just move this file under oak/server?

Copy link
Contributor Author

@rbehjati rbehjati Jun 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this change might break too many things and would be better to be done in a separate PR. But I can give it a try here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are dependency issues... I'll do this in a separate PR.

conradgrobler
conradgrobler approved these changes Jun 10, 2020
View reviewed changes
scripts/run_tests Outdated
@@ -15,7 +15,7 @@ export RUST_BACKTRACE=1

for crate in "${ALL_CRATES[@]}"; do
# Skip the experimental crates. They are covered in `build_experimental`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps update the comment to explain why runner is also being skipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Updated the script. Too many if blocks already.

daviddrysdale
daviddrysdale approved these changes Jun 10, 2020
View reviewed changes
scripts/common Outdated
@@ -17,7 +17,7 @@ readonly SERVER_DOCKER_IMAGE_NAME='gcr.io/oak-ci/oak-server'
readonly CACHE_DIR='bazel-cache'

# List all workspaces, and standalone crates.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any standalone crates left?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: yes, I think so – expect and roughenough.

  • ./Cargo.toml:
    • ./oak/server/rust/oak_loader/Cargo.toml
    • ./oak/server/rust/oak_runtime/Cargo.toml
  • ./examples/Cargo.toml:
    • ./examples/abitest/abitest_common/Cargo.toml
    • ./examples/abitest/module_0/rust/Cargo.toml
    • ./examples/abitest/module_1/rust/Cargo.toml
    • ./examples/abitest/tests/Cargo.toml
    • ./examples/aggregator/backend/Cargo.toml
    • ./examples/aggregator/common/Cargo.toml
    • ./examples/aggregator/module/rust/Cargo.toml
    • ./examples/authentication/client/Cargo.toml
    • ./examples/chat/module/rust/Cargo.toml
    • ./examples/hello_world/module/rust/Cargo.toml
    • ./examples/machine_learning/module/rust/Cargo.toml
    • ./examples/private_set_intersection/module/rust/Cargo.toml
    • ./examples/running_average/module/rust/Cargo.toml
    • ./examples/translator/common/Cargo.toml
    • ./examples/translator/module/rust/Cargo.toml
    • ./examples/trusted_information_retrieval/client/rust/Cargo.toml
    • ./examples/trusted_information_retrieval/module/rust/Cargo.toml
  • ./experimental/Cargo.toml:
    • ./experimental/split_grpc/client/Cargo.toml
    • ./experimental/split_grpc/proxy/Cargo.toml
    • ./experimental/split_grpc/server/Cargo.toml
  • ./oak_abi/Cargo.toml
  • ./oak_utils/Cargo.toml
  • ./runner/Cargo.toml
  • ./sdk/Cargo.toml:
    • ./sdk/rust/oak/Cargo.toml
    • ./sdk/rust/oak_tests/Cargo.toml
  • ./third_party/expect/Cargo.toml
  • ./third_party/roughenough/Cargo.toml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Updated the comment, since expect and roughenough are not currently in the list. Should I add them to the list, and run all the scripts for them too?

@daviddrysdale daviddrysdale mentioned this pull request Jun 10, 2020
Closed
@rbehjati
Move the remaining crates to separate workspaces
1345b66 
* Moved `sdk`, and `runner` to separate workspaces
* Updated scripts so that the checks run for the new workspaces too
@rbehjati rbehjati merged commit 0d4bd45 into project-oak:master Jun 11, 2020
@rbehjati rbehjati deleted the oak-971-rest branch June 11, 2020 09:50
@github-actions
Copy link

Reproducibility Index:

2914fe02bc3443f6131dbc1162c8192a5d4e0864ff7c36f01586cb9bade8e63a  ./examples/target/wasm32-unknown-unknown/release/abitest_0_frontend.wasm
b1b5608763abc792b0a24e044e14e5917609f8a5bbdf1c67882d0c4324070946  ./examples/target/wasm32-unknown-unknown/release/abitest_1_backend.wasm
25d140732267a21be7b102ff81fb7f60de74b73ab598ca94ae7fd0fa09c28355  ./examples/target/wasm32-unknown-unknown/release/aggregator.wasm
bc61f670b87e9a96a66e5e9efcd56c97cb590dee9ae8db6b236b372c093c2583  ./examples/target/wasm32-unknown-unknown/release/chat.wasm
903d9ce0f2a7bdcab33e240c9a843095fd733684d1352eb6c81bdcfc04642481  ./examples/target/wasm32-unknown-unknown/release/hello_world.wasm
0bd3b7a2c3f9e2c7d411cc32b01dce9e21fe792528de385903d5bd7e1de44f60  ./examples/target/wasm32-unknown-unknown/release/machine_learning.wasm
9ec622e6689d0600e655e5dfd3ac30dddd3c9d85341fca343f84ccae73c73f98  ./examples/target/wasm32-unknown-unknown/release/private_set_intersection.wasm
090482036c29de804768c0b6bb8389883308b6c8bb42bdec2719de74183d7528  ./examples/target/wasm32-unknown-unknown/release/running_average.wasm
82f6b7a102adf2e926e5f1546ef0262f699f92c4c25313be97f745c1014d29fe  ./examples/target/wasm32-unknown-unknown/release/translator.wasm
8d548975b7c25a3adfbdcec355b1e48d351818a5e850d4c35c215c3badbe0b2e  ./examples/target/wasm32-unknown-unknown/release/trusted_information_retrieval.wasm
ce9279f19bb5219fbd709501031284832d493740689a2e23c2e984102c0a851c  ./target/x86_64-unknown-linux-musl/release/oak_loader

Reproducibility Index diff:


      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
          

  

    
  


  

      

    @rbehjati
rbehjati



    mentioned this pull request
    
      Jan 11, 2022
    





  


  

      
  
  Closed

  







  













  
  

    

      

  




    


    

      

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



      

    

  




  
          




      

  

    
    

    
  

    Reviewers
  


    


        

          

  
      @tiziano88
  
    tiziano88



          
            
              
            
          
          tiziano88 left review comments

        

        

          

  
      @conradgrobler
  
    conradgrobler



          
            
              
            
          
          conradgrobler approved these changes

        

        

          

  
      @daviddrysdale
  
    daviddrysdale



          
            
              
            
          
          daviddrysdale approved these changes

        


      






    

  


      
  

    Assignees
  



        

    No one assigned







      


  


  

    Labels
  



    

      

    cla: yes









      

  

    

        

    Projects
  


        




    None yet




  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    
      

        
    

    Development
  



          


Successfully merging this pull request may close these issues.



  


    
  




      

    

  
      

  

    

      5 participants
    

    

        
          @rbehjati 
        
          @tiziano88 
        
          @daviddrysdale 
        
          @conradgrobler 
        
          @googlebot