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

[wip] Resource split #17769

Closed
wants to merge 5 commits into from
Closed

[wip] Resource split #17769

wants to merge 5 commits into from

Conversation

@paulrouget
Copy link
Contributor

paulrouget commented Jul 18, 2017

don't review yet

Splitting resources in 3 categories:

  • support/* for files that are not necessary for libservo (usually, for ports/ packaging)
  • tests/resources/ for files used for tests
  • resources/ for files necessary for libservo

This is necessary when one wants to embed Servo. We don't want to have to deal with useless files. And eventually, we will want to package these files within the libservo binary (see #15635).


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #17097 (github issue number if applicable).

This change is Reviewable

@highfive
Copy link

highfive commented Jul 18, 2017

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/testing_commands.py, python/servo/package_commands.py, python/servo/build_commands.py
  • @jgraham: tests/wpt/run.py
@highfive
Copy link

highfive commented Jul 18, 2017

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
@paulrouget paulrouget force-pushed the paulrouget:resource_split branch 2 times, most recently from 0a78595 to 5fba71c Jul 18, 2017
@jdm
Copy link
Member

jdm commented Jul 18, 2017

error[E0425]: cannot find function `resources_dir_path` in this scope

   --> /home/travis/build/servo/servo/tests/unit/net/fetch.rs:158:20

    |

158 |     let mut path = resources_dir_path().expect("Cannot find resource dir");

    |                    ^^^^^^^^^^^^^^^^^^ not found in this scope

    |

help: possible candidate is found in another module, you can import it into scope

    |

157 | fn test_fetch_file() use servo_config::resource_files::resources_dir_path;

    |

error[E0433]: failed to resolve. Use of undeclared type or module `std`

   --> /home/travis/build/servo/servo/tests/unit/net/fetch.rs:520:16

    |

520 |     let path = std::env::current_exe().unwrap().push("tests").push("resources");

    |                ^^^^^^^^^^^^^^^^^^^^^ Use of undeclared type or module `std`

error[E0425]: cannot find function `resources_dir_path` in this scope

   --> /home/travis/build/servo/servo/tests/unit/net/fetch.rs:533:19

    |

533 |     let ca_file = resources_dir_path().unwrap().join("self_signed_certificate_for_testing.crt");

    |                   ^^^^^^^^^^^^^^^^^^ not found in this scope

    |

help: possible candidate is found in another module, you can import it into scope

    |

514 | fn test_fetch_with_hsts() use servo_config::resource_files::resources_dir_path;
@jdm jdm added the S-fails-travis label Jul 18, 2017
@paulrouget paulrouget force-pushed the paulrouget:resource_split branch 2 times, most recently from 3f721f5 to a5f7af8 Jul 19, 2017
@paulrouget paulrouget force-pushed the paulrouget:resource_split branch from a5f7af8 to b5060da Jul 19, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Jul 21, 2017

The latest upstream changes (presumably #17822) made this pull request unmergeable. Please resolve the merge conflicts.

@nox nox assigned jdm and unassigned nox Aug 8, 2017
@jdm jdm removed the S-awaiting-review label Aug 8, 2017
@bors-servo
Copy link
Contributor

bors-servo commented Aug 15, 2017

The latest upstream changes (presumably #17425) made this pull request unmergeable. Please resolve the merge conflicts.

@jdm
Copy link
Member

jdm commented Nov 15, 2017

Closing due to inactivity.

@jdm jdm closed this Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.