git cl "Not uploading ... file is too large."

[Kevin Bailey]

I'm trying to 'git cl upload' a patch to hunspell_dictionaries but it is almost silently skipping the .dic file. Buried in the output is:

Not uploading the patch for ...dic because the file is too large.

It is1,649,856 bytes. I assume that there is a work-around because there are .dic files already there 10x larger. Anyone know? Upload in small pieces? ;)

thx,

krb

[Scott Hess]

My experience is that large files upload the file, without a patch.  The review will show a context diff, rather than a side-by-side.  Commit-queue may work, for an ASCII file, sometimes you have to land manually (git cl land).

-scott

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Kevin Bailey]

Should the CL mention the file at all? This one doesn't.

[Scott Hess]

Hmm.  In this CL:

   https://codereview.chromium.org/1785443004/

amalgamation/sqlite3.c is an example of a file where the patch didn't get uploaded, but the base file did.  Another example would be changes involving tools/metrics/histograms/histograms.xml .

I can't tell if .dic files are binary files, or UTF-8.  If binary, you may have an additional layer of issues, though I thought the problem there was that you couldn't actually commit using the automated tools, not that it didn't work at all.

-scott

[Kevin Bailey]

They're UTF8, and the error message was "too large". I imagine that I could compile up a hacked version of git-cl but I don't know if Rietveld would take it.

[Paweł Hajdan, Jr.]

+infra-dev

[Aaron Gable]

Rietveld does not support files above a certain size. It's not a limitation of git-cl, but of Rietveld itself.

The correct solution here is to:

Locally commit without the diffs to the large files

Upload that as a CL

Get review and lgtm

Locally commit the large files

Manually land the CL, bypassing rietveld+cq

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.
To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/CAATLsPb-KAoUihpU8Q1DfHqvZC1PeG2Lkj2OR0r9j-eo4%3Dgd_A%40mail.gmail.com.

[Kevin Bailey]

That's my plan, but it would still be nice, if not actually correct, if codereview... listed the file at least, and said, "(not uploaded due to size, commit locally)".

[Aaron Gable]

If you want to file that as a feature request against +Andrew Bonventre with the Infra-Codereview label, please go ahead and do so.

[Scott Hess]

Since sqlite3.c is 6.5M and histograms.xml is 3.5M, it would seem that this 1.6M .dic file should be within that size constraint, though.  Unless this changed recently?

-scott

On Fri, Mar 18, 2016 at 10:05 AM, Aaron Gable <aga...@chromium.org> wrote:

[John Abd-El-Malek]

My experience was the same as scottmg. Even in the case of large files, the diff should be uploaded and reviewable, and landable through CQ.

On Fri, Mar 18, 2016 at 10:05 AM, Aaron Gable <aga...@chromium.org> wrote:

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/CAH58R2es7EcUjCaCbwCCMDd1eUPrpf7VgkOjmkaP41JVTRJgJw%40mail.gmail.com.

[Scott Graham]

I did not engage here (/fistshake shess!), but that has been my experience too fwiw, e.g. with tools/metrics/histograms/histograms.xml which is 3.5M.

[John Abd-El-Malek]

argh sorry wrong scott.

Ken: was it https://codereview.chromium.org/1810993003/?  If so, which file specifically are you wondering about?

[John Abd-El-Malek]

On Fri, Mar 18, 2016 at 2:04 PM, John Abd-El-Malek <j...@chromium.org> wrote:

argh sorry wrong scott.

Ken: was it https://codereview.chromium.org/1810993003/?  If so, which file specifically are you wondering about?

uhhm, Kevin I mean :)

[Kevin Bailey]

Yes. You can't see it, but there's a fa_IR.dic file there.

[Scott Hess]

OK, just talked to Jon - there are two bits, the limit for base files (the entire file) and the limit to patches.  I get the impression both limits are 1MB.  Why you can operate with histograms.xml and other large files is that the base file is too big, but the patch fits in the limit.

In this case, you should probably just manually land the new big files.  Since they're new, they presumably won't affect the build.  My general approach in such cases is to land the file in unmodified form from whatever created it, then separately land modifications.  That way you only really need to get informal LGTM on "We actually do want that file to exist", and use the regular review system for the local changes.  Obviously that won't work with large binary files.

-scott

[Kevin Bailey]

'git cl land' lists the file in question, which is good, but landing fails, with no hints:

Total 22 (delta 14), reused 0 (delta 0)

remote: Resolving deltas: 100% (14/14)

remote: Processing changes: refs: 1, done    

error: failed to push some refs to 'https://chromium.googlesource.com/chromium/deps/hunspell_dictionaries.git'

Failed to push. If this persists, please file a bug.

Filed 596869.

[Primiano Tucci]

Posting result for future records:

The issue seems to be krb@ not being in https://chromium-committers.appspot.com/lists/commi...@chromium.org

Somebody has to git cl land on his behalf.

P.S. for infra-dev: would be nice if git cl land somehow did figure that out printing a more descriptive message, e.g., "git cl land failed, are you a committer?"

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/CAONXcN%2BAaV_RKheuEdbPE%3DWQ4Utwn3QcjOGzfz2m8-zK%3D0adDQ%40mail.gmail.com.