21.10.07. chromium issue_1187007 패치 도전(성공)

Nayeon Kim·2021년 10월 7일
2

chromium

목록 보기
9/10

issue 1187007은 GetAsString()을 지우고, is_string(), GetString(), GetIfString으로 대체시키는 작업을 요한다. 이를 codeHealth, code_rotation 작업이라 부르는 듯 한데, 크로미움에는 이러한 함수를 대체하고자 하는 이슈가 다양하게 많이 올라와 있다.

크롬 코드들 중 상당히 많은 파일들에서 GetAsString()을 쓰고 있기 때문에, 여러 사람들이 각각 일부 directory를 맡아서 그 directory 내의 파일들을 수정하는 방식이다. 이 이슈는 21년 5월부터 시작해 패치가 많이 진행된 상태이지만, 그래도 수정할 파일들이 꽤 남아있다.

패치 과정

  1. getAsString()을 사용하고 있는 코드를 찾자.
    예전에는 파일 탐색기를 통해 검색했었는데, 시간이 오래 걸릴 뿐만 아니라 파일 탐색기가 검색할 수 있는 정도에 비해 크롬 코드 양이 상당해서 꼼꼼히 다 찾아내지도 못한다.
    이는, 웹 사이트를 통해 검색하는 것이 효율적이다. 크롬 또한 코드를 검색할 수 있는 웹사이트를 따로 다루고 있다. https://source.chromium.org/ 에서 getAsString()을 검색하면, getAsString()을 사용하고 있는 모든 코드가 뜨고, 어느 위치에 있는지까지도 알 수 있다.
    나는 이 중에서 out/Debug/gen/extensions/common/api/* 하위 파일들을 수정하고자 했다.(근데 수정이 불가능한 디렉토리임을 나중에 알았음.)

  2. 어떻게 코드를 수정해야할지 모르겠다면, 다른 사람들의 패치들을 참고해보자.
    GetAsString 함수를 지우고 is_string(), GetString() 등으로 대체해야 하는 게 목표라면, 일단은 이 함수들이 어떤 일들을 하는지 알아야 했다. 이에 대해서 나는 GetAsString(), is_string(), GetString() 각각 사용하고 있는 코드들을 찾아서 역할이 뭔지 이해해보고자 했다.
    그 이후에는, 같은 이슈의 merge 완료된 패치들에서 어떤 방식으로 수정했는지를 참고했다.
    보통 2가지의 방식을 사용하고 있는 것을 알 수 있었는데 쉽게 예를 들어보면,

<기존>
std::string a;
if (!abc.getAsString(a))
	func(a);



<변경1>
if (!abc.is_string());
	func(abc.getString());
   
<변경2>
const std::string *a = abc.GetIfString();
if (!a)
	func(*a);

나는 이 중에서, <변경1> 방식을 참고해 코드를 수정하고자 했다.
out/Debug/gen/extensions/common/api/의 28개 남짓 되는 파일을 모두 같은 방법으로 수정한 후, commit하기 위해 cmd에 git commit -a를 입력한 순간

warning이 뜨면서 코드 수정 부분이 적용되지 않고 다 reset됐다.
당황해서 구글링해보니 대충 gitignore과 관련되어 나오는 에러인 듯 했는데, gitignore 파일에 들어가 봤을 때 out/
디렉토리 역시 모두 포함돼있었다.
멘토님께서 알려주신 정확한 이유는, out/Debug 디렉토리의 경우 빌드 이후 자동으로 생성되는 파일이라 수정이 따로 불가능하다는 것.

그래서 수정이 가능한 net/log/* 디렉토리 내 파일들을 위의 방법대로 고치고,
아직 merge된 패치가 없어 등록하지 못한 AUTHORS 파일에 내 이름과 메일 정보를 넣어서
review를 보냈다.
보통 리뷰어들로부터 새벽에 답이 오는 경우가 많았는데, 아침 7시에 리뷰를 보내자마자 답변이 왔다. 아직 권한이 없는 나를 대신해 리뷰어가 CQ dry run을 실행해 모든 체제에서 문제가 없음을 확인하고, 마침내 정상적으로 merge가 되었다.
https://chromium-review.googlesource.com/c/chromium/src/+/3202410

(ps. 추가적으로 멍청한 실수. reviewer에게 reply 남길 때 draft 상태면 상대에게 안 보이는 거였다니.. draft가 정말 그 draft 의미였구나 그동안 열심히 혼자 대화하고 있었음..ㅎ)

profile
Department of Computer Science

0개의 댓글